-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent emitting invalid-name
#8337
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it should be raised here, right ?
_foo: str = "tomato" | |
_foo: str = "tomato" # [disallowed-name] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.x Then, create a pull request where the |
β¦v#8337) Closes pylint-dev#8307 Co-authored-by: Pierre Sassoulas <[email protected]>
β¦8354) Closes #8307 Co-authored-by: Mark Byrne <[email protected]>
Prevent emitting
invalid-name
for the line on which aglobal
statement is declared.Type of Changes
Description
The
visit_global
function seems to be redundant because thevisit_assignname
method is already running the self._check_name("const", node.name, node) method on the node representing the module-level assignment.Closes #8307