Skip to content

Fix config migration to check section presence not parse success (#4234)#4234

Closed
anushamukka-dev wants to merge 1 commit into
facebook:mainfrom
anushamukka-dev:export-D111620497
Closed

Fix config migration to check section presence not parse success (#4234)#4234
anushamukka-dev wants to merge 1 commit into
facebook:mainfrom
anushamukka-dev:export-D111620497

Conversation

@anushamukka-dev

@anushamukka-dev anushamukka-dev commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary:

When migrating a pyproject.toml with MigrationSource::Auto, the code ran try_mypy().or_else(|_| try_pyright()). That falls back to pyright on any mypy parse failure, not just "section missing" — so a present-but-malformed [tool.mypy] (bad value types, etc.) silently became a pyright migration whenever both sections were present, hiding the mypy error the user actually needs to fix.

This decides which tool to migrate based on section presence rather than parse success. A new has_toml_section helper does lightweight TOML table-header detection (independent of section validity). Both Args::load_from_pyproject (used by pyrefly init) and the in-memory find_and_migrate_in_memory loader now use it: when [tool.mypy] is present we attempt mypy and surface its error; only when mypy is absent and [tool.pyright] is present do we migrate pyright; the both-absent case preserves prior behavior.

This also removes the now-resolved TODO and the ad-hoc post-hoc error-precedence logic in the in-memory loader, which is subsumed by presence-based selection.

Reviewed By: kinto0

Differential Revision: D111620497

@meta-cla meta-cla Bot added the cla signed label Jul 21, 2026
@meta-codesync

meta-codesync Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

@anushamukka-dev has exported this pull request. If you are a Meta employee, you can view the originating Diff in D111620497.

@kinto0 kinto0 self-assigned this Jul 21, 2026

@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.

@meta-codesync meta-codesync Bot changed the title Fix config migration to check section presence not parse success Fix config migration to check section presence not parse success (#4234) Jul 22, 2026
anushamukka-dev added a commit to anushamukka-dev/pyrefly that referenced this pull request Jul 22, 2026
…ebook#4234)

Summary:

When migrating a `pyproject.toml` with `MigrationSource::Auto`, the code ran `try_mypy().or_else(|_| try_pyright())`. That falls back to pyright on *any* mypy parse failure, not just "section missing" — so a present-but-malformed `[tool.mypy]` (bad value types, etc.) silently became a pyright migration whenever both sections were present, hiding the mypy error the user actually needs to fix.

This decides which tool to migrate based on section *presence* rather than parse *success*. A new `has_toml_section` helper does lightweight TOML table-header detection (independent of section validity). Both `Args::load_from_pyproject` (used by `pyrefly init`) and the in-memory `find_and_migrate_in_memory` loader now use it: when `[tool.mypy]` is present we attempt mypy and surface its error; only when mypy is absent and `[tool.pyright]` is present do we migrate pyright; the both-absent case preserves prior behavior.

This also removes the now-resolved TODO and the ad-hoc post-hoc error-precedence logic in the in-memory loader, which is subsumed by presence-based selection.

Reviewed By: kinto0

Differential Revision: D111620497
@github-actions github-actions Bot added size/m and removed size/m labels Jul 22, 2026
anushamukka-dev added a commit to anushamukka-dev/pyrefly that referenced this pull request Jul 23, 2026
…ebook#4234)

Summary:

When migrating a `pyproject.toml` with `MigrationSource::Auto`, the code ran `try_mypy().or_else(|_| try_pyright())`. That falls back to pyright on *any* mypy parse failure, not just "section missing" — so a present-but-malformed `[tool.mypy]` (bad value types, etc.) silently became a pyright migration whenever both sections were present, hiding the mypy error the user actually needs to fix.

This decides which tool to migrate based on section *presence* rather than parse *success*. A new `has_toml_section` helper does lightweight TOML table-header detection (independent of section validity). Both `Args::load_from_pyproject` (used by `pyrefly init`) and the in-memory `find_and_migrate_in_memory` loader now use it: when `[tool.mypy]` is present we attempt mypy and surface its error; only when mypy is absent and `[tool.pyright]` is present do we migrate pyright; the both-absent case preserves prior behavior.

This also removes the now-resolved TODO and the ad-hoc post-hoc error-precedence logic in the in-memory loader, which is subsumed by presence-based selection.

Reviewed By: kinto0

Differential Revision: D111620497
@github-actions github-actions Bot added size/m and removed size/m labels Jul 23, 2026
…ebook#4234)

Summary:

When migrating a `pyproject.toml` with `MigrationSource::Auto`, the code ran `try_mypy().or_else(|_| try_pyright())`. That falls back to pyright on *any* mypy parse failure, not just "section missing" — so a present-but-malformed `[tool.mypy]` (bad value types, etc.) silently became a pyright migration whenever both sections were present, hiding the mypy error the user actually needs to fix.

This decides which tool to migrate based on section *presence* rather than parse *success*. A new `has_toml_section` helper does lightweight TOML table-header detection (independent of section validity). Both `Args::load_from_pyproject` (used by `pyrefly init`) and the in-memory `find_and_migrate_in_memory` loader now use it: when `[tool.mypy]` is present we attempt mypy and surface its error; only when mypy is absent and `[tool.pyright]` is present do we migrate pyright; the both-absent case preserves prior behavior.

This also removes the now-resolved TODO and the ad-hoc post-hoc error-precedence logic in the in-memory loader, which is subsumed by presence-based selection.

Reviewed By: kinto0

Differential Revision: D111620497
@meta-codesync

meta-codesync Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

This pull request has been merged in fd3e282.

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.

2 participants