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

Resolve classes and functions relative to script name #10965

Merged
merged 3 commits into from Apr 18, 2024

Conversation

charliermarsh
Copy link
Member

Summary

If the user is analyzing a script (i.e., we have no module path), it seems reasonable to use the script name when trying to identify paths to objects defined within the script.

Closes #10960.

Test Plan

Ran:

check --isolated --select=B008 \
    --config 'lint.flake8-bugbear.extend-immutable-calls=["test.A"]' \
    test.py

On:

class A: pass

def f(a=A()):
    pass

@charliermarsh charliermarsh added bug Something isn't working configuration Related to settings and configuration and removed bug Something isn't working labels Apr 16, 2024
Some(resolved)

// If we have a fully-qualified path for the module, use it.
if let Some(path) = self.module.path() {
Copy link
Member Author

Choose a reason for hiding this comment

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

I consider changing Module to return &[script_name] in the script case, but the path is typically used for resolving relative imports, and I'm not sure it makes as much sense for us to resolve relative imports relative to the script name given that it's definitionally a standalone script.

@zanieb
Copy link
Member

zanieb commented Apr 16, 2024

Not worth unit test coverage?

@charliermarsh
Copy link
Member Author

Will try.

Copy link
Contributor

github-actions bot commented Apr 16, 2024

ruff-ecosystem results

Linter (stable)

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

python/typeshed (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select E,F,FA,I,PYI,RUF,UP,W

+ stdlib/typing.pyi:330:10: PYI001 Name of private `TypeVar` must start with `_`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PYI001 1 1 0 0 0

Linter (preview)

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

python/typeshed (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select E,F,FA,I,PYI,RUF,UP,W

+ stdlib/typing.pyi:330:10: PYI001 Name of private `TypeVar` must start with `_`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PYI001 1 1 0 0 0

@charliermarsh
Copy link
Member Author

I think those typeshed changes are arguably good but I'll let @AlexWaygood review.

@AlexWaygood
Copy link
Member

I think those typeshed changes are arguably good but I'll let @AlexWaygood review.

The typing.pyi change is definitely correct — we already flag that in flake8-pyi, and I'd expect it to be flagged.

The builtins hit seems like a false positive though — the existing annotation looks correct there. Something now isn't being inferred as being a builtin symbol because we're actually in builtins.pyi itself?

@AlexWaygood
Copy link
Member

The builtins hit seems like a false positive though — the existing annotation looks correct there. Something now isn't being inferred as being a builtin symbol because we're actually in builtins.pyi itself?

Oh! I missed that the error was going away rather than being introduced! The typeshed hits both look like clear improvements, then 😃

@charliermarsh charliermarsh force-pushed the charlie/script branch 2 times, most recently from 80fa654 to 23565b5 Compare April 16, 2024 17:44
@charliermarsh charliermarsh enabled auto-merge (squash) April 16, 2024 17:45
Copy link

codspeed-hq bot commented Apr 16, 2024

CodSpeed Performance Report

Merging #10965 will not alter performance

Comparing charlie/script (df60061) with main (06b3e37)

Summary

✅ 30 untouched benchmarks

@charliermarsh charliermarsh enabled auto-merge (squash) April 18, 2024 01:41
@charliermarsh charliermarsh merged commit b23414e into main Apr 18, 2024
17 checks passed
@charliermarsh charliermarsh deleted the charlie/script branch April 18, 2024 01:42
@fofoni
Copy link

fofoni commented Apr 18, 2024

Thank you all for handling this so quickly!!

@charliermarsh
Copy link
Member Author

No problem, sorry that it took a few days to merge -- I had to debug a performance regression 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extend-immutable-calls doesn't work on Python scripts (as opposed to modules)
5 participants