[syntax-errors] `nonlocal` declaration at module level by abhijeetbodas2001 · Pull Request #17559 · astral-sh/ruff

abhijeetbodas2001 · 2025-04-22T17:54:56Z

Summary

Part of #17412

Add a new compile-time syntax error for detecting nonlocal declarations at a module level.

Test Plan

Added new inline tests for the syntax error
Updated existing tests for nonlocal statement parsing to be inside a function scope

abhijeetbodas2001 · 2025-04-22T17:55:20Z

@dhruvmanila could you review this?

ntBre

Awesome, thanks! I had one suggestion about where to put tests, but this looks great to me.

crates/ruff_python_parser/src/parser/statement.rs

dhruvmanila · 2025-04-23T15:14:22Z

(I'll leave this to Brent)

abhijeetbodas2001 · 2025-04-23T18:10:11Z

@ntBre Thanks for the review. I've addressed your comment and pushed again.

I have one question -- since it looks like this repo follows the squash+merge git workflow, should I be updating the PR description every time I push after addressing review comments (since this description will end up in the commit message)?

(Edit: updated the PR description)

abhijeetbodas2001 · 2025-04-24T03:55:09Z

(Resolved conflicts in the last push)

ntBre

This looks great, thanks!

ntBre · 2025-04-24T17:19:29Z

I have one question -- since it looks like this repo follows the squash+merge git workflow, should I be updating the PR description every time I push after addressing review comments (since this description will end up in the commit message)?

Great question! I would say not to worry too much about it, but it is definitely nice to keep it updated if things change substantially. Your edits here look perfect.

github-actions · 2025-04-24T17:24:43Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -0 violations, +0 -12 fixes in 1 projects; 54 projects unchanged)

apache/airflow (+0 -0 violations, +0 -12 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

- providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:353:36: AIR311 [*] `airflow.datasets.DatasetAny` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
+ providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:353:36: AIR311 `airflow.datasets.DatasetAny` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
- providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:353:48: AIR311 [*] `airflow.datasets.DatasetAll` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
+ providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:353:48: AIR311 `airflow.datasets.DatasetAll` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
- providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:355:67: AIR311 [*] `airflow.datasets.DatasetAll` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
+ providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:355:67: AIR311 `airflow.datasets.DatasetAll` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
- providers/openlineage/tests/unit/openlineage/plugins/test_utils.py:678:18: AIR311 [*] `airflow.timetables.datasets.DatasetOrTimeSchedule` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
+ providers/openlineage/tests/unit/openlineage/plugins/test_utils.py:678:18: AIR311 `airflow.timetables.datasets.DatasetOrTimeSchedule` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
- providers/openlineage/tests/unit/openlineage/plugins/test_utils.py:774:18: AIR311 [*] `airflow.timetables.datasets.DatasetOrTimeSchedule` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
+ providers/openlineage/tests/unit/openlineage/plugins/test_utils.py:774:18: AIR311 `airflow.timetables.datasets.DatasetOrTimeSchedule` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
- providers/openlineage/tests/unit/openlineage/utils/test_utils.py:985:22: AIR311 [*] `airflow.timetables.datasets.DatasetOrTimeSchedule` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.
+ providers/openlineage/tests/unit/openlineage/utils/test_utils.py:985:22: AIR311 `airflow.timetables.datasets.DatasetOrTimeSchedule` is removed in Airflow 3.0; It still works in Airflow 3.0 but is expected to be removed in a future version.

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
AIR311	12	0	0	0	12

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

abhijeetbodas2001 · 2025-04-25T06:10:05Z

Great question! I would say not to worry too much about it, but it is definitely nice to keep it updated if things change substantially. Your edits here look perfect.

Got it, thanks!

Also wanted to mention that your detailed issue write up was really helpful from a new contributor (ie, me) POV, so thanks for that as well 😄

ntBre · 2025-04-25T12:38:48Z

Thank you, that's great to hear! And thanks for the great PRs!

* main: (24 commits) [`airflow`] Extend `AIR301` rule (#17598) [`airflow`] update existing `AIR302` rules with better suggestions (#17542) red_knot_project: sort diagnostics from checking files [red-knot] fix detecting a metaclass on a not-explicitly-specialized generic base (#17621) [red-knot] fix inheritance-cycle detection for generic classes (#17620) [`pylint`] Detect `global` declarations in module scope (`PLE0118`) (#17411) Add Semantic Error Test for LateFutureImport (#17612) [red-knot] change TypeVarInstance to be interned, not tracked (#17616) [red-knot] Special case `@final`, `@override` (#17608) [red-knot] add TODO comment in specialization code (#17615) [`semantic-syntax-errors`] test for `LoadBeforeGlobalDeclaration` - ruff linter (#17592) [syntax-errors] `nonlocal` declaration at module level (#17559) [`airflow`] Apply auto fix to cases where name has been changed in Airflow 3 (`AIR311`) (#17571) [syntax-errors] Make `async-comprehension-in-sync-comprehension` more specific (#17460) Bump 0.11.7 (#17613) [red-knot] Use iterative approach to collect overloads (#17607) red_knot_python_semantic: avoid Rust's screaming snake case convention in mdtest red_knot_python_semantic: improve diagnostics for unsupported boolean conversions red_knot_python_semantic: add "return type span" helper method red_knot_python_semantic: move parameter span helper method ...

* main: (27 commits) [red-knot] Add new property tests for subtyping with "bottom" callable (#17635) [red-knot] Create generic context for generic classes lazily (#17617) ruff_db: add tests for annotations with no ranges [`airflow`] Extend `AIR301` rule (#17598) [`airflow`] update existing `AIR302` rules with better suggestions (#17542) red_knot_project: sort diagnostics from checking files [red-knot] fix detecting a metaclass on a not-explicitly-specialized generic base (#17621) [red-knot] fix inheritance-cycle detection for generic classes (#17620) [`pylint`] Detect `global` declarations in module scope (`PLE0118`) (#17411) Add Semantic Error Test for LateFutureImport (#17612) [red-knot] change TypeVarInstance to be interned, not tracked (#17616) [red-knot] Special case `@final`, `@override` (#17608) [red-knot] add TODO comment in specialization code (#17615) [`semantic-syntax-errors`] test for `LoadBeforeGlobalDeclaration` - ruff linter (#17592) [syntax-errors] `nonlocal` declaration at module level (#17559) [`airflow`] Apply auto fix to cases where name has been changed in Airflow 3 (`AIR311`) (#17571) [syntax-errors] Make `async-comprehension-in-sync-comprehension` more specific (#17460) Bump 0.11.7 (#17613) [red-knot] Use iterative approach to collect overloads (#17607) red_knot_python_semantic: avoid Rust's screaming snake case convention in mdtest ...

abhijeetbodas2001 requested review from MichaReiser and dhruvmanila as code owners April 22, 2025 17:54

MichaReiser assigned ntBre Apr 23, 2025

ntBre reviewed Apr 23, 2025

View reviewed changes

crates/ruff_python_parser/src/parser/statement.rs Show resolved Hide resolved

dhruvmanila removed their request for review April 23, 2025 15:14

abhijeetbodas2001 force-pushed the apb branch from 6f645d6 to ea605b1 Compare April 23, 2025 18:04

abhijeetbodas2001 requested a review from ntBre April 24, 2025 02:38

[syntax-errors] nonlocal declaration not allowed module level

16dcacd

abhijeetbodas2001 force-pushed the apb branch from ea605b1 to 16dcacd Compare April 24, 2025 03:51

ntBre approved these changes Apr 24, 2025

View reviewed changes

ntBre changed the title ~~[syntax-errors] nonlocal declaration at module level~~ [syntax-errors] nonlocal declaration at module level Apr 24, 2025

ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Apr 24, 2025

ntBre mentioned this pull request Apr 24, 2025

[syntax-errors] Remaining syntax errors raised by the compiler #17412

Open

14 tasks

Merge branch 'main' into apb

8bb451b

ntBre merged commit cf59cee into astral-sh:main Apr 24, 2025
33 checks passed

abhijeetbodas2001 deleted the apb branch April 25, 2025 03:48

BrewTestBot mentioned this pull request May 1, 2025

ruff 0.11.8 Homebrew/homebrew-core#222139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[syntax-errors] `nonlocal` declaration at module level#17559

[syntax-errors] `nonlocal` declaration at module level#17559
ntBre merged 2 commits intoastral-sh:mainfrom
abhijeetbodas2001:apb

abhijeetbodas2001 commented Apr 22, 2025 •

edited

Loading

Uh oh!

abhijeetbodas2001 commented Apr 22, 2025

Uh oh!

ntBre left a comment

Uh oh!

Uh oh!

dhruvmanila commented Apr 23, 2025

Uh oh!

abhijeetbodas2001 commented Apr 23, 2025 •

edited

Loading

Uh oh!

abhijeetbodas2001 commented Apr 24, 2025

Uh oh!

ntBre left a comment

Uh oh!

ntBre commented Apr 24, 2025

Uh oh!

github-actions bot commented Apr 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

abhijeetbodas2001 commented Apr 25, 2025

Uh oh!

ntBre commented Apr 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

abhijeetbodas2001 commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

abhijeetbodas2001 commented Apr 22, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dhruvmanila commented Apr 23, 2025

Uh oh!

abhijeetbodas2001 commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abhijeetbodas2001 commented Apr 24, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre commented Apr 24, 2025

Uh oh!

github-actions bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

Uh oh!

abhijeetbodas2001 commented Apr 25, 2025

Uh oh!

ntBre commented Apr 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abhijeetbodas2001 commented Apr 22, 2025 •

edited

Loading

abhijeetbodas2001 commented Apr 23, 2025 •

edited

Loading

github-actions bot commented Apr 24, 2025 •

edited

Loading

`ruff-ecosystem` results