Skip to content

[pyrefly] Diagnose directory-relative imports - #4905

Open
WilliamK112 wants to merge 4 commits into
facebook:mainfrom
WilliamK112:codex/implicit-relative-import-4174
Open

WilliamK112 wants to merge 4 commits into
facebook:mainfrom
WilliamK112:codex/implicit-relative-import-4174

Conversation

@WilliamK112

@WilliamK112 WilliamK112 commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add a dedicated directory-relative-import diagnostic when an unqualified import resolves only through FallbackSearchPath::DirectoryRelative.

The diagnostic is a non-fatal warning by default, so the module continues to resolve while users get guidance that first emphasizes missing or misconfigured search paths and site package paths, then explicit relative or fully qualified imports. BasedPyright reportImplicitRelativeImport migration keeps the existing missing-import mapping and additionally configures the dedicated error kind.

The branch is merged with current upstream main, uses the current FindImportOptions API, and covers directory-relative fallback, explicit fallback, configured search-path precedence, end-to-end diagnostic reporting, configuration migration, generated presets, and documentation.

Fixes #4174

Test Plan

  • cargo fmt --all -- --check
  • cargo test -p pyrefly_config test_implicit_relative_import_maps_to_import_error_kinds
  • cargo test -p pyrefly_config test_doc_headers
  • cargo test -p pyrefly_config test_doc_severities
  • cargo test -p pyrefly_config test_error_presets_json
  • cargo test -p pyrefly test_directory_relative_import_emits_dedicated_diagnostic
  • cargo test -p pyrefly test_directory_relative_fallback_reports_directory_relative_import
  • cargo test -p pyrefly test_explicit_fallback_does_not_report_directory_relative_import
  • cargo test -p pyrefly test_absolute_search_path_precedes_directory_relative_fallback
  • python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema

Imports that resolve only through the directory-relative fallback can work when a file is run directly but fail when it is loaded as part of a package.

Attach a typed, non-fatal diagnostic at that narrow resolver boundary while preserving normal search precedence and keeping explicit fallback roots silent. Map the BasedPyright compatibility rule to the new error kind, document it, and cover resolver, configuration, and end-to-end behavior.

Signed-off-by: Ching-Wei Kang <ckang53@wisc.edu>
@meta-codesync

meta-codesync Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D119718772. (Because this pull request was imported automatically, there will not be any future comments.)

@github-actions

This comment has been minimized.

@kinto0 kinto0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

@kinto0

kinto0 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

looks great to me, but I wonder if this should default to "ignore" instead of "warn". cc @connernilsen for opinions

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the stale label Sep 29, 2026

@connernilsen connernilsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

Comment thread crates/pyrefly_config/src/migration/pyright.rs Outdated
Comment thread pyrefly/lib/state/loader.rs Outdated
@github-actions github-actions Bot added size/l and removed size/l labels Oct 12, 2026
@WilliamK112

Copy link
Copy Markdown
Contributor Author

Addressed both review threads and rebased the implementation onto current upstream main:

  • Renamed the diagnostic to directory-relative-import / DirectoryRelativeImport so it does not sound like Python relative-import syntax.
  • Reworded the diagnostic and docs to prioritize a missing or misconfigured search path or site package path.
  • Kept reportImplicitRelativeImport mapped to missing-import and added the dedicated diagnostic mapping.
  • Updated the resolver tests to the current FindImportOptions API and verified that the diagnostic is attached only to the directory-relative fallback, not explicit fallback or configured search-path resolution.
  • Updated generated presets and migration documentation.

All targeted tests, documentation consistency tests, formatting, Clippy, and extension tests pass. I also ran the full 9,078-test library suite: 9,072 passed and 3 were ignored. The remaining 3 host-sensitive LSP tests fail locally because interpreter discovery selects /opt/anaconda3; that interpreter has whatthepatch installed and bypasses the synthetic project venv expectations. The pushed change does not touch interpreter discovery or those LSP tests, so CI on the clean runners should be authoritative.

Ready for another look when convenient.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the stale label Oct 13, 2026
@kinto0

kinto0 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

thanks! I think the last remaining item is to default to "warn" instead of "ignore"

@WilliamK112

Copy link
Copy Markdown
Contributor Author

@kinto0 Addressed the last item on head 091db227b: unconfigured projects using the basic preset now report directory-relative-import as a warning instead of suppressing it. I also regenerated scripts/error_presets.json and documented the Basic warning.

Validation:

  • cargo test -p pyrefly_config test_preset
  • cargo test -p pyrefly_config test_error_presets_json
  • cargo test test_directory_relative_import_emits_dedicated_diagnostic
  • python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema --no-extension

All passed. Ready for another look when convenient.

@github-actions github-actions Bot added size/l and removed size/l labels Oct 15, 2026
@github-actions

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a diagnostic for implicit relative imports

3 participants