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

Redirect PLR1701 to SIM101 #12021

Merged
merged 1 commit into from
Jun 25, 2024
Merged

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jun 25, 2024

Summary

This rule removes PLR1701 and redirects it to SIM101.

In addition to that, the SIM101 autofix has been fixed to add padding if required.

PLR1701 has bugs

It also seems that the implementation of PLR1701 is incorrect in multiple scenarios. For example, the following code snippet:

# There are two _different_ variables `a` and `b`
if isinstance(a, int) or isinstance(b, bool) or isinstance(a, float):
    pass
# There's another condition `or 1`
if isinstance(self.k, int) or isinstance(self.k, float) or 1:
    pass

is fixed to:

# Fixed to only considering variable `a`
if isinstance(a, (float, int)):
    pass
# The additional condition is not present in the fix
if isinstance(self.k, (float, int)):
    pass

Playground: https://play.ruff.rs/6cfbdfb7-f183-43b0-b59e-31e728b34190

Documentation Preview

PLR1701

Screenshot 2024-06-25 at 11 14 40

Test Plan

Remove the test cases for PLR1701, port the padding test case to SIM101 and update the snapshot.

@dhruvmanila dhruvmanila added the rule Implementing or modifying a lint rule label Jun 25, 2024
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+0 -11 violations, +0 -0 fixes in 2 projects; 48 projects unchanged)

apache/airflow (+0 -3 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

- airflow/providers/microsoft/azure/hooks/msgraph.py:327:12: PLR1701 [*] Merge `isinstance` calls: `isinstance(data, (BytesIO, bytes, str))`
- airflow/serialization/pydantic/dag.py:45:9: PLR1701 [*] Merge `isinstance` calls
- airflow/serialization/pydantic/taskinstance.py:70:8: PLR1701 [*] Merge `isinstance` calls: `isinstance(x, (BaseOperator, MappedOperator))`

demisto/content (+0 -8 violations, +0 -0 fixes)

- Packs/ANYRUN/Integrations/ANYRUN/ANYRUN.py:196:16: PLR1701 Merge `isinstance` calls: `isinstance(val, dict | list)`
- Packs/AWS-GuardDuty/Integrations/AWSGuardDutyEventCollector/AWSGuardDutyEventCollector.py:33:12: PLR1701 Merge `isinstance` calls: `isinstance(obj, date | datetime)`
- Packs/Base/Scripts/DBotMLFetchData/DBotMLFetchData.py:187:23: PLR1701 Merge `isinstance` calls: `isinstance(x, bool | str)`
- Packs/Base/Scripts/DBotMLFetchData/DBotMLFetchData.py:192:37: PLR1701 Merge `isinstance` calls: `isinstance(v, bool | str)`
- Packs/Base/Scripts/DBotTrainClustering/DBotTrainClustering.py:422:8: PLR1701 Merge `isinstance` calls: `isinstance(obj, list | str)`
- Packs/BluecatAddressManager/Integrations/BluecatAddressManager/BluecatAddressManager.py:256:12: PLR1701 Merge `isinstance` calls
- Packs/ExpanseV2/Scripts/ExpanseEvidenceDynamicSection/ExpanseEvidenceDynamicSection.py:15:8: PLR1701 Merge `isinstance` calls: `isinstance(v, float | int | str)`
- Packs/TOPdesk/Integrations/TOPdesk/TOPdesk.py:577:16: PLR1701 Merge `isinstance` calls: `isinstance(value, bool | str)`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PLR1701 11 0 11 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -11 violations, +0 -0 fixes in 2 projects; 48 projects unchanged)

apache/airflow (+0 -3 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

- airflow/providers/microsoft/azure/hooks/msgraph.py:327:12: PLR1701 [*] Merge `isinstance` calls: `isinstance(data, (BytesIO, bytes, str))`
- airflow/serialization/pydantic/dag.py:45:9: PLR1701 [*] Merge `isinstance` calls
- airflow/serialization/pydantic/taskinstance.py:70:8: PLR1701 [*] Merge `isinstance` calls: `isinstance(x, (BaseOperator, MappedOperator))`

demisto/content (+0 -8 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

- Packs/ANYRUN/Integrations/ANYRUN/ANYRUN.py:196:16: PLR1701 Merge `isinstance` calls: `isinstance(val, dict | list)`
- Packs/AWS-GuardDuty/Integrations/AWSGuardDutyEventCollector/AWSGuardDutyEventCollector.py:33:12: PLR1701 Merge `isinstance` calls: `isinstance(obj, date | datetime)`
- Packs/Base/Scripts/DBotMLFetchData/DBotMLFetchData.py:187:23: PLR1701 Merge `isinstance` calls: `isinstance(x, bool | str)`
- Packs/Base/Scripts/DBotMLFetchData/DBotMLFetchData.py:192:37: PLR1701 Merge `isinstance` calls: `isinstance(v, bool | str)`
- Packs/Base/Scripts/DBotTrainClustering/DBotTrainClustering.py:422:8: PLR1701 Merge `isinstance` calls: `isinstance(obj, list | str)`
- Packs/BluecatAddressManager/Integrations/BluecatAddressManager/BluecatAddressManager.py:256:12: PLR1701 Merge `isinstance` calls
- Packs/ExpanseV2/Scripts/ExpanseEvidenceDynamicSection/ExpanseEvidenceDynamicSection.py:15:8: PLR1701 Merge `isinstance` calls: `isinstance(v, float | int | str)`
- Packs/TOPdesk/Integrations/TOPdesk/TOPdesk.py:577:16: PLR1701 Merge `isinstance` calls: `isinstance(value, bool | str)`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PLR1701 11 0 11 0 0

@dhruvmanila dhruvmanila added this to the v0.5.0 milestone Jun 25, 2024
@dhruvmanila dhruvmanila merged commit f002a25 into ruff-0.5 Jun 25, 2024
20 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/redirect-PLR1701-to-SIM101 branch June 25, 2024 13:51
@MichaReiser MichaReiser mentioned this pull request Jun 26, 2024
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
## Summary

This rule removes `PLR1701` and redirects it to `SIM101`.

In addition to that, the `SIM101` autofix has been fixed to add padding
if required.

### `PLR1701` has bugs

It also seems that the implementation of `PLR1701` is incorrect in
multiple scenarios. For example, the following code snippet:
```py
# There are two _different_ variables `a` and `b`
if isinstance(a, int) or isinstance(b, bool) or isinstance(a, float):
    pass
# There's another condition `or 1`
if isinstance(self.k, int) or isinstance(self.k, float) or 1:
    pass
```
is fixed to:
```py
# Fixed to only considering variable `a`
if isinstance(a, (float, int)):
    pass
# The additional condition is not present in the fix
if isinstance(self.k, (float, int)):
    pass
```

Playground: https://play.ruff.rs/6cfbdfb7-f183-43b0-b59e-31e728b34190

## Documentation Preview

### `PLR1701`

<img width="1397" alt="Screenshot 2024-06-25 at 11 14 40"
src="https://github.com/astral-sh/ruff/assets/67177269/779ee84d-7c4d-4bb8-a3a4-c2b23a313eba">

## Test Plan

Remove the test cases for `PLR1701`, port the padding test case to
`SIM101` and update the snapshot.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
## Summary

This rule removes `PLR1701` and redirects it to `SIM101`.

In addition to that, the `SIM101` autofix has been fixed to add padding
if required.

### `PLR1701` has bugs

It also seems that the implementation of `PLR1701` is incorrect in
multiple scenarios. For example, the following code snippet:
```py
# There are two _different_ variables `a` and `b`
if isinstance(a, int) or isinstance(b, bool) or isinstance(a, float):
    pass
# There's another condition `or 1`
if isinstance(self.k, int) or isinstance(self.k, float) or 1:
    pass
```
is fixed to:
```py
# Fixed to only considering variable `a`
if isinstance(a, (float, int)):
    pass
# The additional condition is not present in the fix
if isinstance(self.k, (float, int)):
    pass
```

Playground: https://play.ruff.rs/6cfbdfb7-f183-43b0-b59e-31e728b34190

## Documentation Preview

### `PLR1701`

<img width="1397" alt="Screenshot 2024-06-25 at 11 14 40"
src="https://github.com/astral-sh/ruff/assets/67177269/779ee84d-7c4d-4bb8-a3a4-c2b23a313eba">

## Test Plan

Remove the test cases for `PLR1701`, port the padding test case to
`SIM101` and update the snapshot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

repeated-isinstance-calls is a duplicate rule of duplicate-isinstance-call
2 participants