Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Detect unused TypeVars #83

Merged
merged 8 commits into from
Jan 16, 2022
Merged

Detect unused TypeVars #83

merged 8 commits into from
Jan 16, 2022

Conversation

AlexWaygood
Copy link
Collaborator

No description provided.

@AlexWaygood AlexWaygood requested a review from Akuli January 16, 2022 15:16
@AlexWaygood AlexWaygood marked this pull request as draft January 16, 2022 15:19
JelleZijlstra
JelleZijlstra reviewed Jan 16, 2022
View reviewed changes
pyi.py Outdated
# Mapping of all private TypeVars/ParamSpecs/TypeVarTuples to the nodes where they're defined
typevarlike_defs: dict[TypeVarInfo, ast.Assign] = field(default_factory=dict)
# Mapping of each name in the file to the no. of occurences
all_name_occurences: Counter[str] = field(default_factory=Counter)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
all_name_occurences: Counter[str] = field(default_factory=Counter)
all_name_occurrences: Counter[str] = field(default_factory=Counter)

This needs to be changed in a few other places

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahh good catch

tests/typevar.pyi Show resolved Hide resolved
@AlexWaygood
Copy link
Collaborator Author

AlexWaygood commented Jan 16, 2022 •
edited
Loading

A few false positives in the typeshed output which I need to sort out.

@JelleZijlstra
Copy link
Collaborator

Looks like we need to unconditionally call generic_visit in visit_BinOp.

@AlexWaygood
Copy link
Collaborator Author

Looks like we need to unconditionally call generic_visit in visit_BinOp.

Yeah, I'm on it.

@Akuli
Copy link
Collaborator

Akuli commented Jan 16, 2022

Looks like we need to unconditionally call generic_visit in visit_BinOp.

This doesn't look right to me, because int | int | int should be only one error, but then it would be two errors: visit_BinOp runs once for int | int | int as a whole, and then separately for the int | int in the beginning. I think the correct fix would be to call self.visit(member) instead of self.generic_visit(member).

@AlexWaygood
Copy link
Collaborator Author

Looks like we need to unconditionally call generic_visit in visit_BinOp.

This doesn't look right to me, because int | int | int should be only one error, but then it would be two errors: visit_BinOp runs once for int | int | int as a whole, and then separately for the int | int in the beginning. I think the correct fix would be to call self.visit(member) instead of self.generic_visit(member).

Yup, that worked -- thanks!

@AlexWaygood AlexWaygood mentioned this pull request Jan 16, 2022
Merged
@AlexWaygood
Copy link
Collaborator Author

python/typeshed#6928 gets rid of the unused TypeVars in Python-2 typeshed.

@AlexWaygood
Copy link
Collaborator Author

I'll extend this to cover unused type aliases in another PR.

@JelleZijlstra JelleZijlstra reopened this Jan 16, 2022
@AlexWaygood AlexWaygood marked this pull request as ready for review January 16, 2022 16:07
JelleZijlstra
JelleZijlstra reviewed Jan 16, 2022
View reviewed changes
pyi.py Outdated Show resolved Hide resolved
AlexWaygood
AlexWaygood commented Jan 16, 2022
View reviewed changes
pyi.py Outdated Show resolved Hide resolved
AlexWaygood
AlexWaygood commented Jan 16, 2022
View reviewed changes
pyi.py Outdated Show resolved Hide resolved
@JelleZijlstra JelleZijlstra merged commit 464c589 into PyCQA:master Jan 16, 2022
@AlexWaygood AlexWaygood deleted the unused-typevars branch January 16, 2022 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

@Akuli Akuli Awaiting requested review from Akuli

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexWaygood @JelleZijlstra @Akuli