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

Emit modified-iterating-{list|dict|set} when iterating literals or using del #6652

Merged
merged 1 commit into from
Jun 4, 2022
Merged

Emit modified-iterating-{list|dict|set} when iterating literals or using del #6652

merged 1 commit into from
Jun 4, 2022

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented May 19, 2022 •
edited
Loading

Type of Changes

Type
✓ 🐛 Bug fix

Description

Closes #6648
Solves an additional false negative: the check wasn't emitted when iterating literals.

@jacobtylerwalls jacobtylerwalls added the False Negative 🦋 No message is emitted but something is wrong with the code label May 19, 2022
@jacobtylerwalls jacobtylerwalls added this to the 2.15.0 milestone May 19, 2022
@coveralls
Copy link

coveralls commented May 19, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2422846935

  • 30 of 32 (93.75%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.007%) to 95.509%
Changes Missing Coverage Covered Lines Changed/Added Lines %
pylint/checkers/modified_iterating_checker.py 19 21 90.48%
Totals Coverage Status
Change from base Build 2422297694: -0.007%
Covered Lines: 16248
Relevant Lines: 17012
💛 - Coveralls

@jacobtylerwalls jacobtylerwalls marked this pull request as draft May 19, 2022 13:34
DanielNoord
DanielNoord reviewed May 19, 2022
View reviewed changes
ChangeLog Outdated Show resolved Hide resolved
pylint/checkers/utils.py Show resolved Hide resolved
Pierre-Sassoulas
Pierre-Sassoulas reviewed May 19, 2022
View reviewed changes
ChangeLog Outdated Show resolved Hide resolved
pylint/checkers/modified_iterating_checker.py Outdated Show resolved Hide resolved
@jacobtylerwalls jacobtylerwalls changed the title Emit modified-iterating{list|dict|set} when using del Emit modified-iterating{list|dict|set} when iterating or literals or using del May 19, 2022
@jacobtylerwalls jacobtylerwalls changed the title Emit modified-iterating{list|dict|set} when iterating or literals or using del Emit modified-iterating{list|dict|set} when iterating literals or using del May 19, 2022
@jacobtylerwalls jacobtylerwalls marked this pull request as ready for review May 19, 2022 16:14
@jacobtylerwalls jacobtylerwalls changed the title Emit modified-iterating{list|dict|set} when iterating literals or using del Emit modified-iterating-{list|dict|set} when iterating literals or using del May 19, 2022
@Pierre-Sassoulas Pierre-Sassoulas self-requested a review May 27, 2022 08:11
DanielNoord
DanielNoord reviewed May 28, 2022
View reviewed changes
doc/whatsnew/2/2.15/full.rst Outdated Show resolved Hide resolved
doc/whatsnew/2/2.15/index.rst Outdated Show resolved Hide resolved
doc/whatsnew/2/2.15/summary.rst Outdated Show resolved Hide resolved
@DanielNoord
Copy link
Collaborator

@jacobtylerwalls Sorry I messed up the merge of main. Got a little overexcited to see if the primer actually works 😄

@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions

This comment has been minimized.

Sign in to view
@DanielNoord
Copy link
Collaborator

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

🎉

Would have been nice to actually find a change 😄

Github not showing the comment because a similar comment has been hidden is annoying though. I wonder if this is a common issue.

@jacobtylerwalls
Copy link
Member Author

I expect more with #6677

@Pierre-Sassoulas
Copy link
Member

Regarding the whatsnew for 2.15 I was thinking of discussing the new format here : https://github.com/PyCQA/pylint/pull/6688/files (i.e. a single file, enforced in CI according tu label)

Let me know what you think :)

@DanielNoord DanielNoord mentioned this pull request May 28, 2022
Closed
@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2022

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

Effect on pandas:
The following messages are now emitted:

  1. redefined-variable-type:
    Redefinition of result type from pandas.core.indexes.datetimes.DatetimeIndex to pandas.core.indexes.multi.MultiIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/multi/test_reshape.py#L126
  2. redefined-variable-type:
    Redefinition of indexer type from pandas.core.indexes.numeric.Int64Index to pandas.core.indexes.datetimes.DatetimeIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/algorithms.py#L1304
  3. too-many-instance-attributes:
    Too many instance attributes (32/11)
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/frame.py#L469

The following messages are no longer emitted:

  1. redefined-variable-type:
    Redefinition of result type from pandas.core.indexes.base.Index to pandas.core.indexes.multi.MultiIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/multi/test_reshape.py#L126
  2. no-member:
    Instance of 'Index' has no 'day_name' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/test_misc.py#L249
  3. no-member:
    Instance of 'Index' has no 'month_name' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/test_misc.py#L274
  4. redefined-variable-type:
    Redefinition of dti type from pandas.core.indexes.datetimes.DatetimeIndex to pandas.core.indexes.base.Index
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/test_misc.py#L248
  5. no-member:
    Instance of 'TimedeltaIndex' has no 'tz_localize' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/test_constructors.py#L180
  6. no-member:
    Instance of 'TimedeltaIndex' has no 'tz_localize' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/test_constructors.py#L181
  7. no-member:
    Instance of 'TimedeltaIndex' has no 'tz_localize' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/test_constructors.py#L187
  8. no-member:
    Instance of 'PeriodIndex' has no 'tz' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/methods/test_shift.py#L101
  9. redefined-variable-type:
    Redefinition of index type from pandas.core.indexes.datetimes.DatetimeIndex to pandas.core.indexes.timedeltas.TimedeltaIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/window/test_rolling.py#L711
  10. no-member:
    Instance of 'int' has no 'groupby' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/groupby/transform/test_transform.py#L1116
  11. redefined-variable-type:
    Redefinition of idx1 type from pandas.core.indexes.period.PeriodIndex to pandas.core.indexes.base.Index
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/frame/methods/test_set_index.py#L497
  12. no-member:
    Instance of 'DataFrame' has no 'one' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/dtypes/test_generic.py#L125
  13. no-member:
    Instance of 'DataFrame' has no 'one' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/dtypes/test_generic.py#L127
  14. redefined-variable-type:
    Redefinition of idx type from pandas.core.indexes.datetimes.DatetimeIndex to pandas.core.indexes.timedeltas.TimedeltaIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/arithmetic/test_datetime64.py#L2302
  15. no-member:
    Instance of 'int' has no 'encoding' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/config/test_localization.py#L107
  16. no-member:
    Instance of 'TimedeltaIndex' has no 'tz_convert' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/io/pytables/test_timezones.py#L229
  17. no-member:
    Instance of 'TimedeltaIndex' has no 'tz_convert' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/io/pytables/test_timezones.py#L230
  18. too-many-instance-attributes:
    Too many instance attributes (33/11)
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/frame.py#L469
  19. attribute-defined-outside-init:
    Attribute 'name' defined outside init
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/frame.py#L9454
  20. redefined-variable-type:
    Redefinition of indexer type from pandas.core.indexes.numeric.Int64Index to pandas.core.indexes.base.Index
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/algorithms.py#L1290

Effect on sentry:
The following messages are now emitted:

  1. no-member:
    Instance of 'Param' has no 'owner' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/sentry_apps/updater.py#L136
  2. no-member:
    Instance of 'Param' has no 'owner' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/sentry_apps/destroyer.py#L38
  3. no-member:
    Instance of 'Param' has no 'owner' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/sentry_apps/destroyer.py#L39
  4. no-member:
    Instance of 'Param' has no 'owner' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/sentry_apps/destroyer.py#L48
  5. import-error:
    Unable to import 'sentry.utils.distutils.commands.base'
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L13
  6. missing-function-docstring:
    Missing function or method docstring
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L42
  7. no-self-use:
    Method could be a function
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L42
  8. too-few-public-methods:
    Too few public methods (1/2)
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L39
  9. import-error:
    Unable to import 'sentry.utils.distutils.commands.base'
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_integration_docs.py#L4
  10. missing-function-docstring:
    Missing function or method docstring
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_integration_docs.py#L10
  11. too-few-public-methods:
    Too few public methods (1/2)
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_integration_docs.py#L7
  12. unused-import:
    Unused Project imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/helpers/group_index/delete.py#L11
  13. unused-import:
    Unused Organization imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/helpers/group_index/index.py#L15
  14. unused-import:
    Unused Project imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/helpers/group_index/index.py#L15

The following messages are no longer emitted:

  1. too-many-instance-attributes:
    Too many instance attributes (12/11)
    https://github.com/getsentry/sentry/blob/master/src/sentry/eventstore/models.py#L34
  2. no-member:
    Instance of 'Param' has no 'schema' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/sentry_app_components/preparer.py#L25
  3. no-member:
    Instance of 'Param' has no 'schema' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/sentry_app_components/preparer.py#L40
  4. no-member:
    Instance of 'Param' has no 'schema' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/sentry_app_components/preparer.py#L58
  5. no-member:
    Instance of 'Param' has no 'name' member
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/external_requests/issue_link_requester.py#L116
  6. unused-import:
    Unused Organization imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/testutils/fixtures.py#L7
  7. not-callable:
    self.get_provider is not callable
    https://github.com/getsentry/sentry/blob/master/src/sentry/models/repository.py#L59

This comment was generated for commit 59fcee7

Pierre-Sassoulas
Pierre-Sassoulas approved these changes Jun 4, 2022
View reviewed changes
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Great !

@Pierre-Sassoulas Pierre-Sassoulas merged commit 8ab9ab5 into pylint-dev:main Jun 4, 2022
@jacobtylerwalls jacobtylerwalls deleted the del-iterating branch June 4, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielNoord DanielNoord DanielNoord approved these changes

@Pierre-Sassoulas Pierre-Sassoulas Pierre-Sassoulas approved these changes

Assignees
No one assigned
Labels
False Negative 🦋 No message is emitted but something is wrong with the code
Projects
None yet
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.

False negative for modified-iterating-list when using the del keyword
4 participants
@jacobtylerwalls @coveralls @DanielNoord @Pierre-Sassoulas