Fix #3299 false positives with names in string literal type annotations #7400

lggruspe · 2022-09-02T13:47:47Z

Type of Changes

	Type
✓	🐛 Bug fix

Description

Closes #3299

Changes:

VariablesChecker now checks for possible names in Const strings that appear in a typing annotation context. This fixes some false positives for unused-import and unused-variable.
string Consts used as args to typing.Literal aren't treated as names.
String annotations like "os.PathLike[str]" or "graphlib.TopologicalSorter[T]" now work

…teral type annotations (pylint-dev#3299)

… typing.Literal arg

Pierre-Sassoulas

Thank you, LGTM, I have only a small question.

tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.py

coveralls · 2022-09-03T06:32:47Z

Pull Request Test Coverage Report for Build 2987707904

37 of 37 (100.0%) changed or added relevant lines in 2 files are covered.
11 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.01%) to 95.328%

Files with Coverage Reduction	New Missed Lines	%
pylint/lint/pylinter.py	11	95.26%

Totals
Change from base Build 2977378583:	0.01%
Covered Lines:	16978
Relevant Lines:	17810

💛 - Coveralls

DanielNoord

Thanks for this @lggruspe.

Some minor stylistic changes but the changes itself look very good!

pylint/checkers/utils.py

pylint/checkers/variables.py

tests/functional/u/unused/unused_name_in_string_literal_type_annotation.py

tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py310.py

tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.py

tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.py

pylint/checkers/utils.py

DanielNoord

For some reason the coverage is going down, but I think that is unrelated to the changes here. Probably coveralls screwing something up.

Thanks @lggruspe! 😄

github-actions · 2022-09-04T10:56:02Z

🤖 Effect of this PR on checked open source code: 🤖

Effect on black:
The following messages are no longer emitted:

unused-import:
Unused Any imported from typing
https://github.com/psf/black/blob/92c93a278036870a76740d5b0b8f06504925e7dc/src/black/concurrency.py#L15
unused-import:
Unused import os
https://github.com/psf/black/blob/92c93a278036870a76740d5b0b8f06504925e7dc/src/blib2to3/pgen2/pgen.py#L22

Effect on psycopg:
The following messages are no longer emitted:

unused-import:
Unused Any imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/cursor_async.py#L8
unused-import:
Unused TupleRow imported from rows
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/connection_async.py#L20
unused-import:
Unused Any imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/_encodings.py#L11
unused-import:
Unused Optional imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/_encodings.py#L11
unused-import:
Unused Sequence imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/_dns.py#L12
unused-import:
Unused Any imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/_adapters_map.py#L7
unused-import:
Unused Any imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/_pipeline.py#L9
unused-import:
Unused ReferenceType imported from weakref
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/connection.py#L13
unused-import:
Unused TupleRow imported from rows
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/connection.py#L26
unused-import:
Unused Type imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/types/net.py#L7
unused-import:
Unused Any imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/types/datetime.py#L10
unused-import:
Unused Type imported from typing
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/crdb/connection.py#L8
unused-import:
Unused TupleRow imported from rows
https://github.com/psycopg/psycopg/blob/5b4b9cc8bddb2ef20af03c773bd8c4efc0db7d90/psycopg/psycopg/crdb/connection.py#L12

Effect on pytest:
The following messages are no longer emitted:

unused-import:
Unused import os
https://github.com/pytest-dev/pytest/blob/4ed2b3a733b0240dfa2260dba98d327213cf04ed/src/_pytest/doctest.py#L4
unused-import:
Unused import os
https://github.com/pytest-dev/pytest/blob/4ed2b3a733b0240dfa2260dba98d327213cf04ed/src/_pytest/compat.py#L5
unused-import:
Unused cached_property imported from functools as cached_property
https://github.com/pytest-dev/pytest/blob/4ed2b3a733b0240dfa2260dba98d327213cf04ed/src/_pytest/compat.py#L356
unused-argument:
Unused argument 'pdb'
https://github.com/pytest-dev/pytest/blob/4ed2b3a733b0240dfa2260dba98d327213cf04ed/src/_pytest/hookspec.py#L953
unused-argument:
Unused argument 'pdb'
https://github.com/pytest-dev/pytest/blob/4ed2b3a733b0240dfa2260dba98d327213cf04ed/src/_pytest/hookspec.py#L964

Effect on sentry:
The following messages are no longer emitted:

unused-import:
Unused User imported from sentry.incidents.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/incidents/charts.py#L16
unused-import:
Unused Future imported from concurrent.futures
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/ingest/ingest_consumer.py#L5
unused-import:
Unused Redis imported from redis
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/utils/kvstore/redis.py#L4
unused-import:
Unused Project imported from sentry.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/api/helpers/group_index/delete.py#L11
unused-import:
Unused Organization imported from sentry.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/api/helpers/group_index/index.py#L15
unused-import:
Unused Project imported from sentry.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/api/helpers/group_index/index.py#L15
unused-import:
Unused User imported from sentry.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/api/helpers/group_index/index.py#L15
unused-import:
Unused Actor imported from sentry.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/api/helpers/group_index/validators/group.py#L6
unused-import:
Unused User imported from sentry.incidents.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/integrations/slack/unfurl/metric_alerts.py#L12
unused-import:
Unused User imported from sentry.models.user
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/integrations/slack/unfurl/discover.py#L16
unused-import:
Unused User imported from sentry.models
https://github.com/getsentry/sentry/blob/f5d9a4c3e403d3419dea9b0005408d34d30c6015/src/sentry/integrations/slack/unfurl/issues.py#L8

This comment was generated for commit 8a562df

Pierre-Sassoulas

Thank you @lggruspe. The changes look good, and the primer results warm my heart.

… annotations (pylint-dev#7400) Don't emit 'unused-variable' or 'unused-import' on names in string literal type annotations (pylint-dev#3299) Don't treat strings inside typing.Literal as names

…ns (#7400) Don't emit 'unused-variable' or 'unused-import' on names in string literal type annotations (#3299) Don't treat strings inside typing.Literal as names

lggruspe added 3 commits September 2, 2022 21:22

Don't emit 'unused-variable' or 'unused-import' on names in string li…

89351a5

…teral type annotations (pylint-dev#3299)

Don't treat strings inside typing.Literal as names

fd6b29e

Create news fragment for pylint-dev#3299

864820c

lggruspe changed the title ~~Issue 3299~~ Fix #3299 false positives with names in string literal type annotations Sep 2, 2022

This comment has been minimized.

Sign in to view

lggruspe added 5 commits September 2, 2022 22:47

Fix broken tests for string names in typing.Literal

985a45e

Fix more broken tests

0fbbddb

Test if unused-variable gets emitted if variable has the same name as…

18522c1

… typing.Literal arg

Test if pylint sees names in return type annotation

84ce348

Check names in string literal type annotations more generally

d1d3d8d

Pierre-Sassoulas reviewed Sep 3, 2022

View reviewed changes

tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.py Show resolved Hide resolved

Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Sep 3, 2022

Pierre-Sassoulas added this to the 2.15.2 milestone Sep 3, 2022

Pierre-Sassoulas added the Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer label Sep 3, 2022

Handle extract_node exceptions

f38f015

DanielNoord requested changes Sep 3, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

lggruspe added 2 commits September 3, 2022 22:16

Stylistic changes

403488d

Return is_typing_literal = False if name lookup fails

63f49d0

lggruspe requested a review from DanielNoord September 3, 2022 14:20

DanielNoord reviewed Sep 3, 2022

View reviewed changes

pylint/checkers/utils.py Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Unit test pylint.checkers.utils.is_typing_literal

8a562df

lggruspe requested a review from DanielNoord September 4, 2022 10:21

DanielNoord approved these changes Sep 4, 2022

View reviewed changes

Pierre-Sassoulas approved these changes Sep 4, 2022

View reviewed changes

Pierre-Sassoulas modified the milestones: 2.15.2, 2.15.1 Sep 4, 2022

Pierre-Sassoulas merged commit 2f766a1 into pylint-dev:main Sep 4, 2022

lggruspe mentioned this pull request Sep 5, 2022

Disambiguate between str and enum member args to typing.Literal #7414

Merged

Pierre-Sassoulas added Backported and removed Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer labels Sep 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3299 false positives with names in string literal type annotations #7400

Fix #3299 false positives with names in string literal type annotations #7400

lggruspe commented Sep 2, 2022 •

edited

This comment has been minimized.

Pierre-Sassoulas left a comment

coveralls commented Sep 3, 2022 •

edited

DanielNoord left a comment

This comment has been minimized.

This comment has been minimized.

DanielNoord left a comment

github-actions bot commented Sep 4, 2022

Pierre-Sassoulas left a comment

Fix #3299 false positives with names in string literal type annotations #7400

Fix #3299 false positives with names in string literal type annotations #7400

Conversation

lggruspe commented Sep 2, 2022 • edited

Type of Changes

Description

This comment has been minimized.

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

coveralls commented Sep 3, 2022 • edited

Pull Request Test Coverage Report for Build 2987707904

💛 - Coveralls

DanielNoord left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

DanielNoord left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 4, 2022

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

lggruspe commented Sep 2, 2022 •

edited

coveralls commented Sep 3, 2022 •

edited