Prevent emitting invalid-name
#8337
Conversation
β¦statement is declared. Closes pylint-dev#8307
| # Prevent emitting `invalid-name` for the line on which `global` is declared | ||
| # https://github.com/PyCQA/pylint/issues/8307 | ||
|
|
||
| _foo: str = "tomato" |
There was a problem hiding this comment.
But it should be raised here, right ?
| _foo: str = "tomato" | |
| _foo: str = "tomato" # [disallowed-name] |
There was a problem hiding this comment.
No because that's the rule we have defined for nodes.Assign vs nodes.AnnAssign. See the big if/elif block here; If it is an AnnAssign, then the const rule-checking is ignored unless it is annotated with Final.
I know the reporter of the issue mentions that it should emit a message for this line but that isn't currently how it works for these module-level assignments. In other words currently, for module-level assignments, _foo: str = "tomato" doesn't emit a message while _foo = "tomato" does.
I'm not saying that behaviour is ideal but I think its valuable to at least remove the checking from the line where global is declared, to prevent duplicating what happens in the visit_assignname method.
There was a problem hiding this comment.
Yeah sorry I'm confused about it, there's a lot of discussion around invalid-name, hard to know what's done or not (#7305). There's also almost too much to digest in https://pylint.readthedocs.io/en/latest/user_guide/messages/convention/invalid-name.html and yet no explanation about Final.
There was a problem hiding this comment.
We have a global-variable-undefined message which will be emitted when the global name is not defined in the module scope. So, when it is defined in the module scope the invalid-name rule-checks are triggered by visit_assignname.
β¦ delaration names a class name in camelCase.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8337 +/- ##
==========================================
- Coverage 95.46% 95.46% -0.01%
==========================================
Files 177 177
Lines 18704 18700 -4
==========================================
- Hits 17856 17852 -4
Misses 848 848
|
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-maintenance/2.16.x maintenance/2.16.x
# Navigate to the new working tree
cd .worktrees/backport-maintenance/2.16.x
# Create a new branch
git switch --create backport-8337-to-maintenance/2.16.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d30c45b2e7a0c13a846c8d221f0c9ff9f012aa4a
# Push it to GitHub
git push --set-upstream origin backport-8337-to-maintenance/2.16.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-maintenance/2.16.xThen, create a pull request where the |
Pierre-Sassoulas
added
the
Needs backport
β¦v#8337) Closes pylint-dev#8307 Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Pierre-Sassoulas
removed
the
Needs backport
Prevent emitting
invalid-namefor the line on which aglobalstatement is declared.Type of Changes
Description
The
visit_globalfunction seems to be redundant because thevisit_assignnamemethod is already running the self._check_name("const", node.name, node) method on the node representing the module-level assignment.Closes #8307