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.

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

Fix consider-using-min-max-builtin #9802

Merged

Conversation

onlined
Copy link
Contributor

@onlined onlined commented Jul 16, 2024

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Fix a false positive for consider-using-min-max-builtin when the assignment target is an attribute.

Closes #9800

Fix a false positive for `consider-using-min-max-builtin` when the
assignment target is an attribute.
@onlined onlined force-pushed the fix-consider-using-min-max-builtin branch from 91a1fe2 to f9330c5 Compare July 16, 2024 12:39
@Pierre-Sassoulas Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Jul 16, 2024
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.2.6 milestone Jul 16, 2024
Copy link

codecov bot commented Jul 16, 2024

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 95.79%. Comparing base (47fb321) to head (f9330c5).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9802   +/-   ##
=======================================
  Coverage   95.78%   95.79%           
=======================================
  Files         174      174           
  Lines       18880    18902   +22     
=======================================
+ Hits        18085    18107   +22     
  Misses        795      795           
Files Coverage Ξ”
pylint/checkers/refactoring/refactoring_checker.py 98.15% <100.00%> (-0.01%) ⬇️

... and 1 file with indirect coverage changes

Copy link
Contributor

πŸ€– Effect of this PR on checked open source code: πŸ€–

Effect on django:
The following messages are now emitted:

  1. consider-using-min-builtin:
    Consider using 'self._envelope.MinX = min(self._envelope.MinX, minx)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L131
  2. consider-using-min-builtin:
    Consider using 'self._envelope.MinY = min(self._envelope.MinY, miny)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L133
  3. consider-using-max-builtin:
    Consider using 'self._envelope.MaxX = max(self._envelope.MaxX, maxx)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L135
  4. consider-using-max-builtin:
    Consider using 'self._envelope.MaxY = max(self._envelope.MaxY, maxy)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L137

The following messages are no longer emitted:

  1. consider-using-min-builtin:
    Consider using 'MinX = min(MinX, minx)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L131
  2. consider-using-min-builtin:
    Consider using 'MinY = min(MinY, miny)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L133
  3. consider-using-max-builtin:
    Consider using 'MaxX = max(MaxX, maxx)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L135
  4. consider-using-max-builtin:
    Consider using 'MaxY = max(MaxY, maxy)' instead of unnecessary if block
    https://github.com/django/django/blob/082fe2b5a83571dec4aa97580af0fe8cf2a5214e/django/contrib/gis/gdal/envelope.py#L137

Effect on sentry:
The following messages are now emitted:

  1. consider-using-max-builtin:
    Consider using 'max_pk = max(max_pk, item.pk)' instead of unnecessary if block
    https://github.com/getsentry/sentry/blob/3f2619d8e23717d55968e5a1f43b45831a393566/src/sentry/backup/services/import_export/impl.py#L529

The following messages are no longer emitted:

  1. consider-using-max-builtin:
    Consider using 'max_pk = max(max_pk, pk)' instead of unnecessary if block
    https://github.com/getsentry/sentry/blob/3f2619d8e23717d55968e5a1f43b45831a393566/src/sentry/backup/services/import_export/impl.py#L529

This comment was generated for commit f9330c5

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 first contribution, the primer result is a pleasure to see πŸŽ‰

@Pierre-Sassoulas Pierre-Sassoulas merged commit 6236b91 into pylint-dev:main Jul 16, 2024
44 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 16, 2024
Fix a false positive for `consider-using-min-max-builtin` when the
assignment target is an attribute.

(cherry picked from commit 6236b91)
Pierre-Sassoulas pushed a commit that referenced this pull request Jul 16, 2024
Fix a false positive for `consider-using-min-max-builtin` when the
assignment target is an attribute.

(cherry picked from commit 6236b91)

Co-authored-by: Ekin Dursun <ekindursun@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport maintenance/3.3.x False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive R1730 consider-using-min-builtin, R1731 consider-using-max-builtin
2 participants