Compare commits

...

35 Commits

Author SHA1 Message Date
Dhruv Manilawala
2db1c0fa96 Use mike for versioned documentation 2024-08-19 13:47:02 +05:30
renovate[bot]
4d0d3b00cb Update rust-wasm-bindgen monorepo (#12975) 2024-08-18 20:44:00 -04:00
renovate[bot]
2be1c4ff04 Update Rust crate syn to v2.0.75 (#12974) 2024-08-18 20:43:54 -04:00
renovate[bot]
edd86d5603 Update Rust crate serde_json to v1.0.125 (#12973) 2024-08-18 20:43:48 -04:00
renovate[bot]
78ad7959ca Update Rust crate serde to v1.0.208 (#12972) 2024-08-18 20:43:42 -04:00
renovate[bot]
d72ecd6ded Update Rust crate ordermap to v0.5.2 (#12971) 2024-08-18 20:43:37 -04:00
renovate[bot]
8617a508bd Update Rust crate libc to v0.2.157 (#12970) 2024-08-18 20:43:31 -04:00
renovate[bot]
c88bd4e884 Update Rust crate ctrlc to v3.4.5 (#12969) 2024-08-18 20:43:24 -04:00
renovate[bot]
fbcda90316 Update Rust crate camino to v1.1.9 (#12967) 2024-08-18 20:43:18 -04:00
renovate[bot]
169d4390cb Update Rust crate clap to v4.5.16 (#12968) 2024-08-18 20:43:05 -04:00
Charlie Marsh
80ade591df Ignore unused arguments on stub functions (#12966)
## Summary

We already enforce this logic for the other `ARG` rules. I'm guessing
this was an oversight.

Closes https://github.com/astral-sh/ruff/issues/12963.
2024-08-18 19:21:33 -04:00
Steve C
4881d32c80 [pylint] - remove AugAssign errors from self-cls-assignment (W0642) (#12957) 2024-08-18 15:31:09 +00:00
Steve C
81a2220ce1 [pylint] - Allow __new__ methods to have cls as their first argument even if decorated with @staticmethod for bad-staticmethod-argument (PLW0211) (#12958) 2024-08-18 16:30:22 +01:00
Aaron Gokaslan
900e98b584 Fix CHANGELOG.md typo (#12955) 2024-08-17 17:43:07 +01:00
Alex Waygood
f9d8189670 [perflint] Improve docs for try-except-in-loop (PERF203) (#12947) 2024-08-17 16:00:15 +01:00
TomerBin
52ba94191a [ruff] Reduce FastAPI false positives in unused-async (RUF029) (#12938) 2024-08-17 14:25:14 +00:00
Micha Reiser
96802d6a7f [pep8-naming] Don't flag from imports following conventional import names (N817) (#12946)
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
2024-08-17 12:05:42 +00:00
Micha Reiser
dd0a7ec73e Pull all types in corpus tests (#12919) 2024-08-17 11:59:55 +00:00
Daniel Sonbolian
25f5ae44c4 [flake8_bugbear] message based on expression location [B015] (#12944) 2024-08-17 13:54:19 +02:00
Alex Waygood
251efe5c41 [ruff] Ignore fstring-missing-syntax (RUF027) for fastAPI paths (#12939)
## Summary

As suggested by @MichaReiser in
https://github.com/astral-sh/ruff/pull/12886#pullrequestreview-2237679793,
this adds an exemption to `RUF027` for `fastAPI` paths, which require
template strings rather than eagerly evaluated f-strings.

## Test Plan

I added a fixture that causes Ruff to emit a false-positive error on
`main` but no longer does with this PR.
2024-08-17 11:10:34 +01:00
Carl Meyer
6359e55383 [red-knot] type narrowing (#12706)
Extend the `UseDefMap` to also track which constraints (provided by e.g.
`if` tests) apply to each visible definition.

Uses a custom `BitSet` and `BitSetArray` to track which constraints
apply to which definitions, while keeping data inline as much as
possible.
2024-08-16 16:34:13 -07:00
Alex Waygood
a9847af6e8 [red-knot] Use Unknown rather than Unbound for unresolved imports (#12932) 2024-08-16 20:10:33 +01:00
Micha Reiser
d61d75d4fa Select stable import name when multiple possible bindings are in scope (#12888) 2024-08-16 20:16:57 +02:00
Alex Waygood
499c0bd875 Bump version to 0.6.1 (#12937)
Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
Co-authored-by: Micha Reiser <micha@reiser.io>
2024-08-16 17:48:06 +01:00
Alex Waygood
4cb30b598f N817 docs: refer to the correct setting (#12935) 2024-08-16 15:41:00 +00:00
Micha Reiser
aba0d83c11 [flake8-naming]: Respect import conventions (N817) (#12922) 2024-08-16 16:28:57 +01:00
Dhruv Manilawala
c319414e54 Ignore blank line rules for docs formatting (#12934)
## Summary

fixes: #12933 

## Test Plan

`python scripts/check_docs_formatted.py --generate-docs`
2024-08-16 15:27:36 +00:00
Alex Waygood
ef1f6d98a0 Fix description of where the contributor list comes from in instructions for making a release (#12931) 2024-08-16 15:37:21 +01:00
Dhruv Manilawala
b850b812de Use cell source code instead of the concatenated one (#12929)
## Summary

fixes: #12880

## Test Plan

Test against the notebook provided in the issue.
2024-08-16 19:50:12 +05:30
Alex Waygood
a87b27c075 [red-knot] Add support for relative imports (#12910)
Co-authored-by: Carl Meyer <carl@astral.sh>
2024-08-16 12:35:27 +01:00
Micha Reiser
9b73532b11 [flake8-async] Fix examples to use async with (#12924) 2024-08-16 12:24:59 +02:00
Alex Waygood
d8debb7a36 Simplify logic for RUF027 (#12907)
## Summary

This PR is a pure refactor to simplify some of the logic for `RUF027`.
This will make it easier to file some followup PRs to help reduce the
false positives from this rule. I'm separating the refactor out into a
separate PR so it's easier to review, and so I can double-check from the
ecosystem report that this doesn't have any user-facing impact.

## Test Plan

`cargo test -p ruff_linter --lib`
2024-08-16 08:05:15 +01:00
Dhruv Manilawala
bd4a947b29 [red-knot] Add symbol and definition for parameters (#12862)
## Summary

This PR adds support for adding symbols and definitions for function and
lambda parameters to the semantic index.

### Notes

* The default expression of a parameter is evaluated in the enclosing
scope (not the type parameter or function scope).
* The annotation expression of a parameter is evaluated in the type
parameter scope if they're present other in the enclosing scope.
* The symbols and definitions are added in the function parameter scope.

### Type Inference

There are two definitions `Parameter` and `ParameterWithDefault` and
their respective `*_definition` methods on the type inference builder.
These methods are preferred and are re-used when checking from a
different region.

## Test Plan

Add test case for validating that the parameters are defined in the
function / lambda scope.

### Benchmark update

Validated the difference in diagnostics for benchmark code between
`main` and this branch. All of them are either directly or indirectly
referencing one of the function parameters. The diff is in the PR description.
2024-08-16 10:59:59 +05:30
Matthieu LAURENT
f121f8b31b [fastapi] Implement fast-api-unused-path-parameter (FAST003) (#12638)
This adds the `fast-api-unused-path-parameter` lint rule, as described
in #12632.

I'm still pretty new to rust, so the code can probably be improved, feel
free to tell me if there's any changes i should make.

Also, i needed to add the `add_parameter` edit function, not sure if it
was in the scope of the PR or if i should've made another one.
2024-08-16 01:46:35 +00:00
Carl Meyer
80efb865e9 [red-knot] fix lookups of possibly-shadowed builtins (#12898)
If a builtin is conditionally shadowed by a global, we didn't correctly
fall back to builtins for the not-defined-in-globals path (see added
test for an example.)
2024-08-15 14:09:29 -07:00
67 changed files with 3116 additions and 607 deletions

View File

@@ -36,28 +36,30 @@ jobs:
version="${{ (inputs.plan != '' && fromJson(inputs.plan).announcement_tag) || inputs.ref }}"
# if version is missing, exit with error
if [[ -z "$version" ]]; then
echo "Can't build docs without a version."
exit 1
echo "Can't build docs without a version."
exit 1
fi
# Use version as display name for now
display_name="$version"
echo "version=$version" >> $GITHUB_ENV
echo "display_name=$display_name" >> $GITHUB_ENV
# Extract the major and minor part of the version for the docs
docs_version="$(echo -n "$version" | cut -d "." -f 1-2)"
echo "version=$version" >> "$GITHUB_ENV"
echo "docs_version=$docs_version" >> "$GITHUB_ENV"
echo "display_name=$display_name" >> "$GITHUB_ENV"
- name: "Set branch name"
run: |
version="${{ env.version }}"
display_name="${{ env.display_name }}"
timestamp="$(date +%s)"
# create branch_display_name from display_name by replacing all
# Create `branch_display_name` from `display_name` by replacing all
# characters disallowed in git branch names with hyphens
branch_display_name="$(echo "$display_name" | tr -c '[:alnum:]._' '-' | tr -s '-')"
branch_display_name="$(echo -n "$display_name" | tr -c '[:alnum:]._' '-' | tr -s '-')"
echo "branch_name=update-docs-$branch_display_name-$timestamp" >> $GITHUB_ENV
echo "timestamp=$timestamp" >> $GITHUB_ENV
echo "branch_name=update-docs-$branch_display_name-$timestamp" >> "$GITHUB_ENV"
echo "timestamp=$timestamp" >> "$GITHUB_ENV"
- name: "Add SSH key"
if: ${{ env.MKDOCS_INSIDERS_SSH_KEY_EXISTS == 'true' }}
@@ -70,7 +72,7 @@ jobs:
- uses: Swatinem/rust-cache@v2
- name: "Install Insiders dependencies"
- name: "Install insiders dependencies"
if: ${{ env.MKDOCS_INSIDERS_SSH_KEY_EXISTS == 'true' }}
run: pip install -r docs/requirements-insiders.txt
@@ -78,74 +80,86 @@ jobs:
if: ${{ env.MKDOCS_INSIDERS_SSH_KEY_EXISTS != 'true' }}
run: pip install -r docs/requirements.txt
- name: "Copy README File"
- name: "Fetch docs repo"
run: |
remote_name="astral-docs"
git remote add "$remote_name" https://${{ secrets.ASTRAL_DOCS_PAT }}@github.com/astral-sh/docs.git
git fetch astral-docs "main:$branch_name"
echo "remote_name=$remote_name" >> "$GITHUB_ENV"
- name: "Configure git"
run: |
git config user.name "astral-docs-bot"
git config user.email "176161322+astral-docs-bot@users.noreply.github.com"
- name: "Transform README and generate docs"
run: |
python scripts/transform_readme.py --target mkdocs
python scripts/generate_mkdocs.py
- name: "Build Insiders docs"
if: ${{ env.MKDOCS_INSIDERS_SSH_KEY_EXISTS == 'true' }}
run: mkdocs build --strict -f mkdocs.insiders.yml
run: |
mike deploy \
--remote "$remote_name" \
--branch "$branch_name" \
--message "Update ruff documentation for $version" \
--config-file mkdocs.insiders.yml \
--update-aliases \
--push \
"$docs_version" latest
- name: "Build docs"
if: ${{ env.MKDOCS_INSIDERS_SSH_KEY_EXISTS != 'true' }}
run: mkdocs build --strict -f mkdocs.public.yml
- name: "Clone docs repo"
run: |
version="${{ env.version }}"
git clone https://${{ secrets.ASTRAL_DOCS_PAT }}@github.com/astral-sh/docs.git astral-docs
- name: "Copy docs"
run: rm -rf astral-docs/site/ruff && mkdir -p astral-docs/site && cp -r site/ruff astral-docs/site/
- name: "Commit docs"
working-directory: astral-docs
run: |
branch_name="${{ env.branch_name }}"
git config user.name "astral-docs-bot"
git config user.email "176161322+astral-docs-bot@users.noreply.github.com"
git checkout -b $branch_name
git add site/ruff
git commit -m "Update ruff documentation for $version"
mike deploy \
--remote "$remote_name" \
--branch "$branch_name" \
--message "Update ruff documentation for $version" \
--config-file mkdocs.public.yml \
--update-aliases \
--push \
"$docs_version" latest
- name: "Create Pull Request"
working-directory: astral-docs
env:
GITHUB_TOKEN: ${{ secrets.ASTRAL_DOCS_PAT }}
run: |
version="${{ env.version }}"
display_name="${{ env.display_name }}"
branch_name="${{ env.branch_name }}"
# Set the docs repository
astral_docs_repo="astral-sh/docs"
# set the PR title
# Set the PR title
pull_request_title="Update ruff documentation for $display_name"
# Delete any existing pull requests that are open for this version
# by checking against pull_request_title because the new PR will
# by checking against `pull_request_title` because the new PR will
# supersede the old one.
gh pr list --state open --json title --jq '.[] | select(.title == "$pull_request_title") | .number' | \
xargs -I {} gh pr close {}
gh pr list \
--state open \
--json title,number \
--jq '.[] | select(.title == "$pull_request_title") | .number' \
--repo "$astral_docs_repo" | \
xargs -I {} gh pr close {}
# push the branch to GitHub
git push origin $branch_name
# create the PR
gh pr create --base main --head $branch_name \
# Create the PR, the branch has already been pushed by `mike`
gh pr create --base main --head "$branch_name" \
--title "$pull_request_title" \
--body "Automated documentation update for $display_name" \
--label "documentation"
--label "documentation" \
--repo "$astral_docs_repo"
- name: "Merge Pull Request"
if: ${{ inputs.plan != '' && !fromJson(inputs.plan).announcement_tag_is_implicit }}
working-directory: astral-docs
env:
GITHUB_TOKEN: ${{ secrets.ASTRAL_DOCS_PAT }}
run: |
branch_name="${{ env.branch_name }}"
# auto-merge the PR if the build was triggered by a release. Manual builds should be reviewed by a human.
# give the PR a few seconds to be created before trying to auto-merge it
sleep 10
gh pr merge --squash $branch_name
# TODO(dhruvmanila): Uncomment once a patch and minor release are done, thus
# confirming that it works as intended
#
# - name: "Merge Pull Request"
# if: ${{ inputs.plan != '' && !fromJson(inputs.plan).announcement_tag_is_implicit }}
# env:
# GITHUB_TOKEN: ${{ secrets.ASTRAL_DOCS_PAT }}
# run: |
# branch_name="${{ env.branch_name }}"
# # auto-merge the PR if the build was triggered by a release. Manual builds should be reviewed by a human.
# # give the PR a few seconds to be created before trying to auto-merge it
# sleep 10
# gh pr merge --squash $branch_name --repo "astral-sh/docs"

View File

@@ -1,5 +1,29 @@
# Changelog
## 0.6.1
This is a hotfix release to address an issue with `ruff-pre-commit`. In v0.6,
Ruff changed its behavior to lint and format Jupyter notebooks by default;
however, due to an oversight, these files were still excluded by default if
Ruff was run via pre-commit, leading to inconsistent behavior.
This has [now been fixed](https://github.com/astral-sh/ruff-pre-commit/pull/96).
### Preview features
- \[`fastapi`\] Implement `fast-api-unused-path-parameter` (`FAST003`) ([#12638](https://github.com/astral-sh/ruff/pull/12638))
### Rule changes
- \[`pylint`\] Rename `too-many-positional` to `too-many-positional-arguments` (`R0917`) ([#12905](https://github.com/astral-sh/ruff/pull/12905))
### Server
- Fix crash when applying "fix-all" code-action to notebook cells ([#12929](https://github.com/astral-sh/ruff/pull/12929))
### Other changes
- \[`flake8-naming`\]: Respect import conventions (`N817`) ([#12922](https://github.com/astral-sh/ruff/pull/12922))
## 0.6.0
Check out the [blog post](https://astral.sh/blog/ruff-v0.6.0) for a migration guide and overview of the changes!
@@ -37,7 +61,7 @@ The following rules have been stabilized and are no longer in preview:
- [`invalid-bytes-return-type`](https://docs.astral.sh/ruff/rules/invalid-bytes-return-type/) (`PLE0308`)
- [`invalid-hash-return-type`](https://docs.astral.sh/ruff/rules/invalid-hash-return-type/) (`PLE0309`)
- [`invalid-index-return-type`](https://docs.astral.sh/ruff/rules/invalid-index-return-type/) (`PLE0305`)
- [`invalid-length-return-type`](https://docs.astral.sh/ruff/rules/invalid-length-return-type/) (`E303`)
- [`invalid-length-return-type`](https://docs.astral.sh/ruff/rules/invalid-length-return-type/) (`PLEE303`)
- [`self-or-cls-assignment`](https://docs.astral.sh/ruff/rules/self-or-cls-assignment/) (`PLW0642`)
- [`byte-string-usage`](https://docs.astral.sh/ruff/rules/byte-string-usage/) (`PYI057`)
- [`duplicate-literal-member`](https://docs.astral.sh/ruff/rules/duplicate-literal-member/) (`PYI062`)

View File

@@ -361,7 +361,7 @@ even patch releases may contain [non-backwards-compatible changes](https://semve
downstream jobs manually if needed.
1. Verify the GitHub release:
1. The Changelog should match the content of `CHANGELOG.md`
1. Append the contributors from the `bump.sh` script
1. Append the contributors from the `scripts/release.sh` script
1. If needed, [update the schemastore](https://github.com/astral-sh/ruff/blob/main/scripts/update_schemastore.py).
1. One can determine if an update is needed when
`git diff old-version-tag new-version-tag -- ruff.schema.json` returns a non-empty diff.

122
Cargo.lock generated
View File

@@ -228,9 +228,9 @@ dependencies = [
[[package]]
name = "camino"
version = "1.1.7"
version = "1.1.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e0ec6b951b160caa93cc0c7b209e5a3bff7aae9062213451ac99493cd844c239"
checksum = "8b96ec4966b5813e2c0507c1f86115c8c5abaadc3980879c3424042a02fd1ad3"
[[package]]
name = "cast"
@@ -270,6 +270,12 @@ version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e"
[[package]]
name = "cfg_aliases"
version = "0.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724"
[[package]]
name = "chic"
version = "1.2.2"
@@ -320,9 +326,9 @@ dependencies = [
[[package]]
name = "clap"
version = "4.5.15"
version = "4.5.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "11d8838454fda655dafd3accb2b6e2bea645b9e4078abe84a22ceb947235c5cc"
checksum = "ed6719fffa43d0d87e5fd8caeab59be1554fb028cd30edc88fc4369b17971019"
dependencies = [
"clap_builder",
"clap_derive",
@@ -395,7 +401,7 @@ version = "3.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2f8c93eb5f77c9050c7750e14f13ef1033a40a0aac70c6371535b6763a01438c"
dependencies = [
"nix",
"nix 0.28.0",
"terminfo",
"thiserror",
"which",
@@ -612,12 +618,12 @@ checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7"
[[package]]
name = "ctrlc"
version = "3.4.4"
version = "3.4.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "672465ae37dc1bc6380a6547a8883d5dd397b0f1faaad4f265726cc7042a5345"
checksum = "90eeab0aa92f3f9b4e87f258c72b139c207d251f9cbc1080a0086b86a8870dd3"
dependencies = [
"nix",
"windows-sys 0.52.0",
"nix 0.29.0",
"windows-sys 0.59.0",
]
[[package]]
@@ -1047,9 +1053,9 @@ dependencies = [
[[package]]
name = "indexmap"
version = "2.3.0"
version = "2.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "de3fc2e30ba82dd1b3911c8de1ffc143c74a914a14e99514d7637e3099df5ea0"
checksum = "93ead53efc7ea8ed3cfb0c79fc8023fbb782a5432b52830b6518941cebe6505c"
dependencies = [
"equivalent",
"hashbrown",
@@ -1215,9 +1221,9 @@ checksum = "8b23360e99b8717f20aaa4598f5a6541efbe30630039fbc7706cf954a87947ae"
[[package]]
name = "js-sys"
version = "0.3.69"
version = "0.3.70"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "29c15563dc2726973df627357ce0c9ddddbea194836909d655df6a75d2cf296d"
checksum = "1868808506b929d7b0cfa8f75951347aa71bb21144b7791bae35d9bccfcfe37a"
dependencies = [
"wasm-bindgen",
]
@@ -1250,9 +1256,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
[[package]]
name = "libc"
version = "0.2.155"
version = "0.2.157"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "97b3888a4aecf77e811145cadf6eef5901f4782c53886191b2f693f24761847c"
checksum = "374af5f94e54fa97cf75e945cce8a6b201e88a1a07e688b47dfd2a59c66dbd86"
[[package]]
name = "libcst"
@@ -1388,6 +1394,16 @@ dependencies = [
"libmimalloc-sys",
]
[[package]]
name = "minicov"
version = "0.3.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5c71e683cd655513b99affab7d317deb690528255a0d5f717f1024093c12b169"
dependencies = [
"cc",
"walkdir",
]
[[package]]
name = "minimal-lexical"
version = "0.2.1"
@@ -1438,7 +1454,19 @@ checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4"
dependencies = [
"bitflags 2.6.0",
"cfg-if",
"cfg_aliases",
"cfg_aliases 0.1.1",
"libc",
]
[[package]]
name = "nix"
version = "0.29.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46"
dependencies = [
"bitflags 2.6.0",
"cfg-if",
"cfg_aliases 0.2.1",
"libc",
]
@@ -1525,9 +1553,9 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d"
[[package]]
name = "ordermap"
version = "0.5.1"
version = "0.5.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8c81974681ab4f0cc9fe49cad56f821d1cc67a08cd2caa9b5d58b0adaa5dd36d"
checksum = "61d7d835be600a7ac71b24e39c92fe6fad9e818b3c71bfc379e3ba65e327d77f"
dependencies = [
"indexmap",
]
@@ -1904,6 +1932,8 @@ dependencies = [
"ruff_text_size",
"rustc-hash 2.0.0",
"salsa",
"smallvec",
"static_assertions",
"tempfile",
"tracing",
"walkdir",
@@ -2060,7 +2090,7 @@ dependencies = [
[[package]]
name = "ruff"
version = "0.6.0"
version = "0.6.1"
dependencies = [
"anyhow",
"argfile",
@@ -2252,7 +2282,7 @@ dependencies = [
[[package]]
name = "ruff_linter"
version = "0.6.0"
version = "0.6.1"
dependencies = [
"aho-corasick",
"annotate-snippets 0.9.2",
@@ -2572,7 +2602,7 @@ dependencies = [
[[package]]
name = "ruff_wasm"
version = "0.6.0"
version = "0.6.1"
dependencies = [
"console_error_panic_hook",
"console_log",
@@ -2799,9 +2829,9 @@ checksum = "1c107b6f4780854c8b126e228ea8869f4d7b71260f962fefb57b996b8959ba6b"
[[package]]
name = "serde"
version = "1.0.206"
version = "1.0.208"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5b3e4cd94123dd520a128bcd11e34d9e9e423e7e3e50425cb1b4b1e3549d0284"
checksum = "cff085d2cb684faa248efb494c39b68e522822ac0de72ccf08109abde717cfb2"
dependencies = [
"serde_derive",
]
@@ -2819,9 +2849,9 @@ dependencies = [
[[package]]
name = "serde_derive"
version = "1.0.206"
version = "1.0.208"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fabfb6138d2383ea8208cf98ccf69cdfb1aff4088460681d84189aa259762f97"
checksum = "24008e81ff7613ed8e5ba0cfaf24e2c2f1e5b8a0495711e44fcd4882fca62bcf"
dependencies = [
"proc-macro2",
"quote",
@@ -2841,9 +2871,9 @@ dependencies = [
[[package]]
name = "serde_json"
version = "1.0.124"
version = "1.0.125"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "66ad62847a56b3dba58cc891acd13884b9c61138d330c0d7b6181713d4fce38d"
checksum = "83c8e735a073ccf5be70aa8066aa984eaf2fa000db6c8d0100ae605b366d31ed"
dependencies = [
"itoa",
"memchr",
@@ -3002,9 +3032,9 @@ checksum = "81cdd64d312baedb58e21336b31bc043b77e01cc99033ce76ef539f78e965ebc"
[[package]]
name = "syn"
version = "2.0.74"
version = "2.0.75"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1fceb41e3d546d0bd83421d3409b1460cc7444cd389341a4c880fe7a042cb3d7"
checksum = "f6af063034fc1935ede7be0122941bafa9bacb949334d090b77ca98b5817c7d9"
dependencies = [
"proc-macro2",
"quote",
@@ -3526,19 +3556,20 @@ checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423"
[[package]]
name = "wasm-bindgen"
version = "0.2.92"
version = "0.2.93"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4be2531df63900aeb2bca0daaaddec08491ee64ceecbee5076636a3b026795a8"
checksum = "a82edfc16a6c469f5f44dc7b571814045d60404b55a0ee849f9bcfa2e63dd9b5"
dependencies = [
"cfg-if",
"once_cell",
"wasm-bindgen-macro",
]
[[package]]
name = "wasm-bindgen-backend"
version = "0.2.92"
version = "0.2.93"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "614d787b966d3989fa7bb98a654e369c762374fd3213d212cfc0251257e747da"
checksum = "9de396da306523044d3302746f1208fa71d7532227f15e347e2d93e4145dd77b"
dependencies = [
"bumpalo",
"log",
@@ -3551,9 +3582,9 @@ dependencies = [
[[package]]
name = "wasm-bindgen-futures"
version = "0.4.42"
version = "0.4.43"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "76bc14366121efc8dbb487ab05bcc9d346b3b5ec0eaa76e46594cabbe51762c0"
checksum = "61e9300f63a621e96ed275155c108eb6f843b6a26d053f122ab69724559dc8ed"
dependencies = [
"cfg-if",
"js-sys",
@@ -3563,9 +3594,9 @@ dependencies = [
[[package]]
name = "wasm-bindgen-macro"
version = "0.2.92"
version = "0.2.93"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a1f8823de937b71b9460c0c34e25f3da88250760bec0ebac694b49997550d726"
checksum = "585c4c91a46b072c92e908d99cb1dcdf95c5218eeb6f3bf1efa991ee7a68cccf"
dependencies = [
"quote",
"wasm-bindgen-macro-support",
@@ -3573,9 +3604,9 @@ dependencies = [
[[package]]
name = "wasm-bindgen-macro-support"
version = "0.2.92"
version = "0.2.93"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e94f17b526d0a461a191c78ea52bbce64071ed5c04c9ffe424dcb38f74171bb7"
checksum = "afc340c74d9005395cf9dd098506f7f44e38f2b4a21c6aaacf9a105ea5e1e836"
dependencies = [
"proc-macro2",
"quote",
@@ -3586,18 +3617,19 @@ dependencies = [
[[package]]
name = "wasm-bindgen-shared"
version = "0.2.92"
version = "0.2.93"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "af190c94f2773fdb3729c55b007a722abb5384da03bc0986df4c289bf5567e96"
checksum = "c62a0a307cb4a311d3a07867860911ca130c3494e8c2719593806c08bc5d0484"
[[package]]
name = "wasm-bindgen-test"
version = "0.3.42"
version = "0.3.43"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d9bf62a58e0780af3e852044583deee40983e5886da43a271dd772379987667b"
checksum = "68497a05fb21143a08a7d24fc81763384a3072ee43c44e86aad1744d6adef9d9"
dependencies = [
"console_error_panic_hook",
"js-sys",
"minicov",
"scoped-tls",
"wasm-bindgen",
"wasm-bindgen-futures",
@@ -3606,9 +3638,9 @@ dependencies = [
[[package]]
name = "wasm-bindgen-test-macro"
version = "0.3.42"
version = "0.3.43"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b7f89739351a2e03cb94beb799d47fb2cac01759b40ec441f7de39b00cbf7ef0"
checksum = "4b8220be1fa9e4c889b30fd207d4906657e7e90b12e0e6b0c8b8d8709f5de021"
dependencies = [
"proc-macro2",
"quote",

View File

@@ -136,8 +136,8 @@ curl -LsSf https://astral.sh/ruff/install.sh | sh
powershell -c "irm https://astral.sh/ruff/install.ps1 | iex"
# For a specific version.
curl -LsSf https://astral.sh/ruff/0.6.0/install.sh | sh
powershell -c "irm https://astral.sh/ruff/0.6.0/install.ps1 | iex"
curl -LsSf https://astral.sh/ruff/0.6.1/install.sh | sh
powershell -c "irm https://astral.sh/ruff/0.6.1/install.ps1 | iex"
```
You can also install Ruff via [Homebrew](https://formulae.brew.sh/formula/ruff), [Conda](https://anaconda.org/conda-forge/ruff),
@@ -170,7 +170,7 @@ Ruff can also be used as a [pre-commit](https://pre-commit.com/) hook via [`ruff
```yaml
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.6.0
rev: v0.6.1
hooks:
# Run the linter.
- id: ruff

View File

@@ -29,6 +29,8 @@ salsa = { workspace = true }
tracing = { workspace = true }
rustc-hash = { workspace = true }
hashbrown = { workspace = true }
smallvec = { workspace = true }
static_assertions = { workspace = true }
[build-dependencies]
path-slash = { workspace = true }

View File

@@ -168,6 +168,24 @@ impl ModuleName {
};
Some(Self(name))
}
/// Extend `self` with the components of `other`
///
/// # Examples
///
/// ```
/// use red_knot_python_semantic::ModuleName;
///
/// let mut module_name = ModuleName::new_static("foo").unwrap();
/// module_name.extend(&ModuleName::new_static("bar").unwrap());
/// assert_eq!(&module_name, "foo.bar");
/// module_name.extend(&ModuleName::new_static("baz.eggs.ham").unwrap());
/// assert_eq!(&module_name, "foo.bar.baz.eggs.ham");
/// ```
pub fn extend(&mut self, other: &ModuleName) {
self.0.push('.');
self.0.push_str(other);
}
}
impl Deref for ModuleName {

View File

@@ -2,7 +2,7 @@ use std::iter::FusedIterator;
pub(crate) use module::Module;
pub use resolver::resolve_module;
pub(crate) use resolver::SearchPaths;
pub(crate) use resolver::{file_to_module, SearchPaths};
use ruff_db::system::SystemPath;
pub use typeshed::vendored_typeshed_stubs;

View File

@@ -77,3 +77,9 @@ pub enum ModuleKind {
/// A python package (`foo/__init__.py` or `foo/__init__.pyi`)
Package,
}
impl ModuleKind {
pub const fn is_package(self) -> bool {
matches!(self, ModuleKind::Package)
}
}

View File

@@ -16,10 +16,9 @@ use crate::semantic_index::expression::Expression;
use crate::semantic_index::symbol::{
FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopedSymbolId, SymbolTable,
};
use crate::semantic_index::use_def::UseDefMap;
use crate::Db;
pub(crate) use self::use_def::UseDefMap;
pub mod ast_ids;
mod builder;
pub mod definition;
@@ -27,6 +26,8 @@ pub mod expression;
pub mod symbol;
mod use_def;
pub(crate) use self::use_def::{DefinitionWithConstraints, DefinitionWithConstraintsIterator};
type SymbolMap = hashbrown::HashMap<ScopedSymbolId, (), ()>;
/// Returns the semantic index for `file`.
@@ -310,12 +311,29 @@ mod tests {
use ruff_text_size::{Ranged, TextRange};
use crate::db::tests::TestDb;
use crate::semantic_index::ast_ids::HasScopedUseId;
use crate::semantic_index::definition::DefinitionKind;
use crate::semantic_index::symbol::{FileScopeId, Scope, ScopeKind, SymbolTable};
use crate::semantic_index::ast_ids::{HasScopedUseId, ScopedUseId};
use crate::semantic_index::definition::{Definition, DefinitionKind};
use crate::semantic_index::symbol::{
FileScopeId, Scope, ScopeKind, ScopedSymbolId, SymbolTable,
};
use crate::semantic_index::use_def::UseDefMap;
use crate::semantic_index::{global_scope, semantic_index, symbol_table, use_def_map};
use crate::Db;
impl UseDefMap<'_> {
fn first_public_definition(&self, symbol: ScopedSymbolId) -> Option<Definition<'_>> {
self.public_definitions(symbol)
.next()
.map(|constrained_definition| constrained_definition.definition)
}
fn first_use_definition(&self, use_id: ScopedUseId) -> Option<Definition<'_>> {
self.use_definitions(use_id)
.next()
.map(|constrained_definition| constrained_definition.definition)
}
}
struct TestCase {
db: TestDb,
file: File,
@@ -374,9 +392,7 @@ mod tests {
let foo = global_table.symbol_id_by_name("foo").unwrap();
let use_def = use_def_map(&db, scope);
let [definition] = use_def.public_definitions(foo) else {
panic!("expected one definition");
};
let definition = use_def.first_public_definition(foo).unwrap();
assert!(matches!(definition.node(&db), DefinitionKind::Import(_)));
}
@@ -411,13 +427,13 @@ mod tests {
);
let use_def = use_def_map(&db, scope);
let [definition] = use_def.public_definitions(
global_table
.symbol_id_by_name("foo")
.expect("symbol to exist"),
) else {
panic!("expected one definition");
};
let definition = use_def
.first_public_definition(
global_table
.symbol_id_by_name("foo")
.expect("symbol to exist"),
)
.unwrap();
assert!(matches!(
definition.node(&db),
DefinitionKind::ImportFrom(_)
@@ -438,11 +454,9 @@ mod tests {
"a symbol used but not defined in a scope should have only the used flag"
);
let use_def = use_def_map(&db, scope);
let [definition] =
use_def.public_definitions(global_table.symbol_id_by_name("x").expect("symbol exists"))
else {
panic!("expected one definition");
};
let definition = use_def
.first_public_definition(global_table.symbol_id_by_name("x").expect("symbol exists"))
.unwrap();
assert!(matches!(
definition.node(&db),
DefinitionKind::Assignment(_)
@@ -477,11 +491,9 @@ y = 2
assert_eq!(names(&class_table), vec!["x"]);
let use_def = index.use_def_map(class_scope_id);
let [definition] =
use_def.public_definitions(class_table.symbol_id_by_name("x").expect("symbol exists"))
else {
panic!("expected one definition");
};
let definition = use_def
.first_public_definition(class_table.symbol_id_by_name("x").expect("symbol exists"))
.unwrap();
assert!(matches!(
definition.node(&db),
DefinitionKind::Assignment(_)
@@ -515,19 +527,116 @@ y = 2
assert_eq!(names(&function_table), vec!["x"]);
let use_def = index.use_def_map(function_scope_id);
let [definition] = use_def.public_definitions(
function_table
.symbol_id_by_name("x")
.expect("symbol exists"),
) else {
panic!("expected one definition");
};
let definition = use_def
.first_public_definition(
function_table
.symbol_id_by_name("x")
.expect("symbol exists"),
)
.unwrap();
assert!(matches!(
definition.node(&db),
DefinitionKind::Assignment(_)
));
}
#[test]
fn function_parameter_symbols() {
let TestCase { db, file } = test_case(
"
def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
pass
",
);
let index = semantic_index(&db, file);
let global_table = symbol_table(&db, global_scope(&db, file));
assert_eq!(names(&global_table), vec!["f", "str", "int"]);
let [(function_scope_id, _function_scope)] = index
.child_scopes(FileScopeId::global())
.collect::<Vec<_>>()[..]
else {
panic!("Expected a function scope")
};
let function_table = index.symbol_table(function_scope_id);
assert_eq!(
names(&function_table),
vec!["a", "b", "c", "args", "d", "kwargs"],
);
let use_def = index.use_def_map(function_scope_id);
for name in ["a", "b", "c", "d"] {
let definition = use_def
.first_public_definition(
function_table
.symbol_id_by_name(name)
.expect("symbol exists"),
)
.unwrap();
assert!(matches!(
definition.node(&db),
DefinitionKind::ParameterWithDefault(_)
));
}
for name in ["args", "kwargs"] {
let definition = use_def
.first_public_definition(
function_table
.symbol_id_by_name(name)
.expect("symbol exists"),
)
.unwrap();
assert!(matches!(definition.node(&db), DefinitionKind::Parameter(_)));
}
}
#[test]
fn lambda_parameter_symbols() {
let TestCase { db, file } = test_case("lambda a, b, c=1, *args, d=2, **kwargs: None");
let index = semantic_index(&db, file);
let global_table = symbol_table(&db, global_scope(&db, file));
assert!(names(&global_table).is_empty());
let [(lambda_scope_id, _lambda_scope)] = index
.child_scopes(FileScopeId::global())
.collect::<Vec<_>>()[..]
else {
panic!("Expected a lambda scope")
};
let lambda_table = index.symbol_table(lambda_scope_id);
assert_eq!(
names(&lambda_table),
vec!["a", "b", "c", "args", "d", "kwargs"],
);
let use_def = index.use_def_map(lambda_scope_id);
for name in ["a", "b", "c", "d"] {
let definition = use_def
.first_public_definition(
lambda_table.symbol_id_by_name(name).expect("symbol exists"),
)
.unwrap();
assert!(matches!(
definition.node(&db),
DefinitionKind::ParameterWithDefault(_)
));
}
for name in ["args", "kwargs"] {
let definition = use_def
.first_public_definition(
lambda_table.symbol_id_by_name(name).expect("symbol exists"),
)
.unwrap();
assert!(matches!(definition.node(&db), DefinitionKind::Parameter(_)));
}
}
/// Test case to validate that the comprehension scope is correctly identified and that the target
/// variable is defined only in the comprehension scope and not in the global scope.
#[test]
@@ -594,9 +703,7 @@ y = 2
let element_use_id =
element.scoped_use_id(&db, comprehension_scope_id.to_scope_id(&db, file));
let [definition] = use_def.use_definitions(element_use_id) else {
panic!("expected one definition")
};
let definition = use_def.first_use_definition(element_use_id).unwrap();
let DefinitionKind::Comprehension(comprehension) = definition.node(&db) else {
panic!("expected generator definition")
};
@@ -693,13 +800,13 @@ def func():
assert_eq!(names(&func2_table), vec!["y"]);
let use_def = index.use_def_map(FileScopeId::global());
let [definition] = use_def.public_definitions(
global_table
.symbol_id_by_name("func")
.expect("symbol exists"),
) else {
panic!("expected one definition");
};
let definition = use_def
.first_public_definition(
global_table
.symbol_id_by_name("func")
.expect("symbol exists"),
)
.unwrap();
assert!(matches!(definition.node(&db), DefinitionKind::Function(_)));
}
@@ -800,9 +907,7 @@ class C[T]:
};
let x_use_id = x_use_expr_name.scoped_use_id(&db, scope);
let use_def = use_def_map(&db, scope);
let [definition] = use_def.use_definitions(x_use_id) else {
panic!("expected one definition");
};
let definition = use_def.first_use_definition(x_use_id).unwrap();
let DefinitionKind::Assignment(assignment) = definition.node(&db) else {
panic!("should be an assignment definition")
};

View File

@@ -8,6 +8,7 @@ use ruff_index::IndexVec;
use ruff_python_ast as ast;
use ruff_python_ast::name::Name;
use ruff_python_ast::visitor::{walk_expr, walk_stmt, Visitor};
use ruff_python_ast::AnyParameterRef;
use crate::ast_node_ref::AstNodeRef;
use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey;
@@ -155,7 +156,7 @@ impl<'db> SemanticIndexBuilder<'db> {
self.current_use_def_map_mut().restore(state);
}
fn flow_merge(&mut self, state: &FlowSnapshot) {
fn flow_merge(&mut self, state: FlowSnapshot) {
self.current_use_def_map_mut().merge(state);
}
@@ -195,9 +196,16 @@ impl<'db> SemanticIndexBuilder<'db> {
definition
}
fn add_constraint(&mut self, constraint_node: &ast::Expr) -> Expression<'db> {
let expression = self.add_standalone_expression(constraint_node);
self.current_use_def_map_mut().record_constraint(expression);
expression
}
/// Record an expression that needs to be a Salsa ingredient, because we need to infer its type
/// standalone (type narrowing tests, RHS of an assignment.)
fn add_standalone_expression(&mut self, expression_node: &ast::Expr) {
fn add_standalone_expression(&mut self, expression_node: &ast::Expr) -> Expression<'db> {
let expression = Expression::new(
self.db,
self.file,
@@ -210,6 +218,7 @@ impl<'db> SemanticIndexBuilder<'db> {
);
self.expressions_by_node
.insert(expression_node.into(), expression);
expression
}
fn with_type_params(
@@ -301,6 +310,23 @@ impl<'db> SemanticIndexBuilder<'db> {
}
}
fn declare_parameter(&mut self, parameter: AnyParameterRef) {
let symbol =
self.add_or_update_symbol(parameter.name().id().clone(), SymbolFlags::IS_DEFINED);
let definition = self.add_definition(symbol, parameter);
if let AnyParameterRef::NonVariadic(with_default) = parameter {
// Insert a mapping from the parameter to the same definition.
// This ensures that calling `HasTy::ty` on the inner parameter returns
// a valid type (and doesn't panic)
self.definitions_by_node.insert(
DefinitionNodeRef::from(AnyParameterRef::Variadic(&with_default.parameter)).key(),
definition,
);
}
}
pub(super) fn build(mut self) -> SemanticIndex<'db> {
let module = self.module;
self.visit_body(module.suite());
@@ -368,6 +394,16 @@ where
.add_or_update_symbol(function_def.name.id.clone(), SymbolFlags::IS_DEFINED);
self.add_definition(symbol, function_def);
// The default value of the parameters needs to be evaluated in the
// enclosing scope.
for default in function_def
.parameters
.iter_non_variadic_params()
.filter_map(|param| param.default.as_deref())
{
self.visit_expr(default);
}
self.with_type_params(
NodeWithScopeRef::FunctionTypeParameters(function_def),
function_def.type_params.as_deref(),
@@ -378,6 +414,12 @@ where
}
builder.push_scope(NodeWithScopeRef::Function(function_def));
// Add symbols and definitions for the parameters to the function scope.
for parameter in &*function_def.parameters {
builder.declare_parameter(parameter);
}
builder.visit_body(&function_def.body);
builder.pop_scope()
},
@@ -456,6 +498,7 @@ where
ast::Stmt::If(node) => {
self.visit_expr(&node.test);
let pre_if = self.flow_snapshot();
self.add_constraint(&node.test);
self.visit_body(&node.body);
let mut post_clauses: Vec<FlowSnapshot> = vec![];
for clause in &node.elif_else_clauses {
@@ -468,7 +511,7 @@ where
self.visit_elif_else_clause(clause);
}
for post_clause_state in post_clauses {
self.flow_merge(&post_clause_state);
self.flow_merge(post_clause_state);
}
let has_else = node
.elif_else_clauses
@@ -477,7 +520,7 @@ where
if !has_else {
// if there's no else clause, then it's possible we took none of the branches,
// and the pre_if state can reach here
self.flow_merge(&pre_if);
self.flow_merge(pre_if);
}
}
ast::Stmt::While(node) => {
@@ -495,13 +538,13 @@ where
// We may execute the `else` clause without ever executing the body, so merge in
// the pre-loop state before visiting `else`.
self.flow_merge(&pre_loop);
self.flow_merge(pre_loop);
self.visit_body(&node.orelse);
// Breaking out of a while loop bypasses the `else` clause, so merge in the break
// states after visiting `else`.
for break_state in break_states {
self.flow_merge(&break_state);
self.flow_merge(break_state);
}
}
ast::Stmt::Break(_) => {
@@ -574,9 +617,25 @@ where
}
ast::Expr::Lambda(lambda) => {
if let Some(parameters) = &lambda.parameters {
// The default value of the parameters needs to be evaluated in the
// enclosing scope.
for default in parameters
.iter_non_variadic_params()
.filter_map(|param| param.default.as_deref())
{
self.visit_expr(default);
}
self.visit_parameters(parameters);
}
self.push_scope(NodeWithScopeRef::Lambda(lambda));
// Add symbols and definitions for the parameters to the lambda scope.
if let Some(parameters) = &lambda.parameters {
for parameter in &**parameters {
self.declare_parameter(parameter);
}
}
self.visit_expr(lambda.body.as_ref());
}
ast::Expr::If(ast::ExprIf {
@@ -591,7 +650,7 @@ where
let post_body = self.flow_snapshot();
self.flow_restore(pre_if);
self.visit_expr(orelse);
self.flow_merge(&post_body);
self.flow_merge(post_body);
}
ast::Expr::ListComp(
list_comprehension @ ast::ExprListComp {
@@ -654,6 +713,14 @@ where
self.pop_scope();
}
}
fn visit_parameters(&mut self, parameters: &'ast ruff_python_ast::Parameters) {
// Intentionally avoid walking default expressions, as we handle them in the enclosing
// scope.
for parameter in parameters.iter().map(ast::AnyParameterRef::as_parameter) {
self.visit_parameter(parameter);
}
}
}
#[derive(Copy, Clone, Debug)]

View File

@@ -45,6 +45,7 @@ pub(crate) enum DefinitionNodeRef<'a> {
Assignment(AssignmentDefinitionNodeRef<'a>),
AnnotatedAssignment(&'a ast::StmtAnnAssign),
Comprehension(ComprehensionDefinitionNodeRef<'a>),
Parameter(ast::AnyParameterRef<'a>),
}
impl<'a> From<&'a ast::StmtFunctionDef> for DefinitionNodeRef<'a> {
@@ -95,6 +96,12 @@ impl<'a> From<ComprehensionDefinitionNodeRef<'a>> for DefinitionNodeRef<'a> {
}
}
impl<'a> From<ast::AnyParameterRef<'a>> for DefinitionNodeRef<'a> {
fn from(node: ast::AnyParameterRef<'a>) -> Self {
Self::Parameter(node)
}
}
#[derive(Copy, Clone, Debug)]
pub(crate) struct ImportFromDefinitionNodeRef<'a> {
pub(crate) node: &'a ast::StmtImportFrom,
@@ -150,6 +157,14 @@ impl DefinitionNodeRef<'_> {
first,
})
}
DefinitionNodeRef::Parameter(parameter) => match parameter {
ast::AnyParameterRef::Variadic(parameter) => {
DefinitionKind::Parameter(AstNodeRef::new(parsed, parameter))
}
ast::AnyParameterRef::NonVariadic(parameter) => {
DefinitionKind::ParameterWithDefault(AstNodeRef::new(parsed, parameter))
}
},
}
}
@@ -168,6 +183,10 @@ impl DefinitionNodeRef<'_> {
}) => target.into(),
Self::AnnotatedAssignment(node) => node.into(),
Self::Comprehension(ComprehensionDefinitionNodeRef { node, first: _ }) => node.into(),
Self::Parameter(node) => match node {
ast::AnyParameterRef::Variadic(parameter) => parameter.into(),
ast::AnyParameterRef::NonVariadic(parameter) => parameter.into(),
},
}
}
}
@@ -182,6 +201,8 @@ pub enum DefinitionKind {
Assignment(AssignmentDefinitionKind),
AnnotatedAssignment(AstNodeRef<ast::StmtAnnAssign>),
Comprehension(ComprehensionDefinitionKind),
Parameter(AstNodeRef<ast::Parameter>),
ParameterWithDefault(AstNodeRef<ast::ParameterWithDefault>),
}
#[derive(Clone, Debug)]
@@ -227,6 +248,10 @@ impl AssignmentDefinitionKind {
pub(crate) fn assignment(&self) -> &ast::StmtAssign {
self.assignment.node()
}
pub(crate) fn target(&self) -> &ast::ExprName {
self.target.node()
}
}
#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)]
@@ -273,3 +298,15 @@ impl From<&ast::Comprehension> for DefinitionNodeKey {
Self(NodeKey::from_node(node))
}
}
impl From<&ast::Parameter> for DefinitionNodeKey {
fn from(node: &ast::Parameter) -> Self {
Self(NodeKey::from_node(node))
}
}
impl From<&ast::ParameterWithDefault> for DefinitionNodeKey {
fn from(node: &ast::ParameterWithDefault) -> Self {
Self(NodeKey::from_node(node))
}
}

View File

@@ -21,7 +21,7 @@ pub(crate) struct Expression<'db> {
/// The expression node.
#[no_eq]
#[return_ref]
pub(crate) node: AstNodeRef<ast::Expr>,
pub(crate) node_ref: AstNodeRef<ast::Expr>,
#[no_eq]
count: countme::Count<Expression<'static>>,

View File

@@ -1,4 +1,5 @@
//! Build a map from each use of a symbol to the definitions visible from that use.
//! Build a map from each use of a symbol to the definitions visible from that use, and the
//! type-narrowing constraints that apply to each definition.
//!
//! Let's take this code sample:
//!
@@ -6,7 +7,7 @@
//! x = 1
//! x = 2
//! y = x
//! if flag:
//! if y is not None:
//! x = 3
//! else:
//! x = 4
@@ -34,8 +35,8 @@
//! [`AstIds`](crate::semantic_index::ast_ids::AstIds) we number all uses (that means a `Name` node
//! with `Load` context) so we have a `ScopedUseId` to efficiently represent each use.
//!
//! The other case we need to handle is when a symbol is referenced from a different scope (the
//! most obvious example of this is an import). We call this "public" use of a symbol. So the other
//! Another case we need to handle is when a symbol is referenced from a different scope (the most
//! obvious example of this is an import). We call this "public" use of a symbol. So the other
//! question we need to be able to answer is, what are the publicly-visible definitions of each
//! symbol?
//!
@@ -53,42 +54,55 @@
//! start.)
//!
//! So this means that the publicly-visible definitions of a symbol are the definitions still
//! visible at the end of the scope.
//! visible at the end of the scope; effectively we have an implicit "use" of every symbol at the
//! end of the scope.
//!
//! The data structure we build to answer these two questions is the `UseDefMap`. It has a
//! We also need to know, for a given definition of a symbol, what type-narrowing constraints apply
//! to it. For instance, in this code sample:
//!
//! ```python
//! x = 1 if flag else None
//! if x is not None:
//! y = x
//! ```
//!
//! At the use of `x` in `y = x`, the visible definition of `x` is `1 if flag else None`, which
//! would infer as the type `Literal[1] | None`. But the constraint `x is not None` dominates this
//! use, which means we can rule out the possibility that `x` is `None` here, which should give us
//! the type `Literal[1]` for this use.
//!
//! The data structure we build to answer these questions is the `UseDefMap`. It has a
//! `definitions_by_use` vector indexed by [`ScopedUseId`] and a `public_definitions` vector
//! indexed by [`ScopedSymbolId`]. The values in each of these vectors are (in principle) a list of
//! visible definitions at that use, or at the end of the scope for that symbol.
//! visible definitions at that use, or at the end of the scope for that symbol, with a list of the
//! dominating constraints for each of those definitions.
//!
//! In order to avoid vectors-of-vectors and all the allocations that would entail, we don't
//! actually store these "list of visible definitions" as a vector of [`Definition`] IDs. Instead,
//! the values in `definitions_by_use` and `public_definitions` are a [`Definitions`] struct that
//! keeps a [`Range`] into a third vector of [`Definition`] IDs, `all_definitions`. The trick with
//! this representation is that it requires that the definitions visible at any given use of a
//! symbol are stored sequentially in `all_definitions`.
//! In order to avoid vectors-of-vectors-of-vectors and all the allocations that would entail, we
//! don't actually store these "list of visible definitions" as a vector of [`Definition`].
//! Instead, the values in `definitions_by_use` and `public_definitions` are a [`SymbolState`]
//! struct which uses bit-sets to track definitions and constraints in terms of
//! [`ScopedDefinitionId`] and [`ScopedConstraintId`], which are indices into the `all_definitions`
//! and `all_constraints` indexvecs in the [`UseDefMap`].
//!
//! There is another special kind of possible "definition" for a symbol: it might be unbound in the
//! scope. (This isn't equivalent to "zero visible definitions", since we may go through an `if`
//! that has a definition for the symbol, leaving us with one visible definition, but still also
//! the "unbound" possibility, since we might not have taken the `if` branch.)
//! There is another special kind of possible "definition" for a symbol: there might be a path from
//! the scope entry to a given use in which the symbol is never bound.
//!
//! The simplest way to model "unbound" would be as an actual [`Definition`] itself: the initial
//! visible [`Definition`] for each symbol in a scope. But actually modeling it this way would
//! dramatically increase the number of [`Definition`] that Salsa must track. Since "unbound" is a
//! unnecessarily increase the number of [`Definition`] that Salsa must track. Since "unbound" is a
//! special definition in that all symbols share it, and it doesn't have any additional per-symbol
//! state, we can represent it more efficiently: we use the `may_be_unbound` boolean on the
//! [`Definitions`] struct. If this flag is `true`, it means the symbol/use really has one
//! additional visible "definition", which is the unbound state. If this flag is `false`, it means
//! we've eliminated the possibility of unbound: every path we've followed includes a definition
//! for this symbol.
//! state, and constraints are irrelevant to it, we can represent it more efficiently: we use the
//! `may_be_unbound` boolean on the [`SymbolState`] struct. If this flag is `true`, it means the
//! symbol/use really has one additional visible "definition", which is the unbound state. If this
//! flag is `false`, it means we've eliminated the possibility of unbound: every path we've
//! followed includes a definition for this symbol.
//!
//! To build a [`UseDefMap`], the [`UseDefMapBuilder`] is notified of each new use and definition
//! as they are encountered by the
//! To build a [`UseDefMap`], the [`UseDefMapBuilder`] is notified of each new use, definition, and
//! constraint as they are encountered by the
//! [`SemanticIndexBuilder`](crate::semantic_index::builder::SemanticIndexBuilder) AST visit. For
//! each symbol, the builder tracks the currently-visible definitions for that symbol. When we hit
//! a use of a symbol, it records the currently-visible definitions for that symbol as the visible
//! definitions for that use. When we reach the end of the scope, it records the currently-visible
//! definitions for each symbol as the public definitions of that symbol.
//! each symbol, the builder tracks the `SymbolState` for that symbol. When we hit a use of a
//! symbol, it records the current state for that symbol for that use. When we reach the end of the
//! scope, it records the state for each symbol as the public definitions of that symbol.
//!
//! Let's walk through the above example. Initially we record for `x` that it has no visible
//! definitions, and may be unbound. When we see `x = 1`, we record that as the sole visible
@@ -98,10 +112,11 @@
//!
//! Then we hit the `if` branch. We visit the `test` node (`flag` in this case), since that will
//! happen regardless. Then we take a pre-branch snapshot of the currently visible definitions for
//! all symbols, which we'll need later. Then we go ahead and visit the `if` body. When we see `x =
//! 3`, it replaces `x = 2` as the sole visible definition of `x`. At the end of the `if` body, we
//! take another snapshot of the currently-visible definitions; we'll call this the post-if-body
//! snapshot.
//! all symbols, which we'll need later. Then we record `flag` as a possible constraint on the
//! currently visible definition (`x = 2`), and go ahead and visit the `if` body. When we see `x =
//! 3`, it replaces `x = 2` (constrained by `flag`) as the sole visible definition of `x`. At the
//! end of the `if` body, we take another snapshot of the currently-visible definitions; we'll call
//! this the post-if-body snapshot.
//!
//! Now we need to visit the `else` clause. The conditions when entering the `else` clause should
//! be the pre-if conditions; if we are entering the `else` clause, we know that the `if` test
@@ -125,98 +140,142 @@
//! (In the future we may have some other questions we want to answer as well, such as "is this
//! definition used?", which will require tracking a bit more info in our map, e.g. a "used" bit
//! for each [`Definition`] which is flipped to true when we record that definition for a use.)
use self::symbol_state::{
ConstraintIdIterator, DefinitionIdWithConstraintsIterator, ScopedConstraintId,
ScopedDefinitionId, SymbolState,
};
use crate::semantic_index::ast_ids::ScopedUseId;
use crate::semantic_index::definition::Definition;
use crate::semantic_index::expression::Expression;
use crate::semantic_index::symbol::ScopedSymbolId;
use ruff_index::IndexVec;
use std::ops::Range;
/// All definitions that can reach a given use of a name.
mod bitset;
mod symbol_state;
/// Applicable definitions and constraints for every use of a name.
#[derive(Debug, PartialEq, Eq)]
pub(crate) struct UseDefMap<'db> {
// TODO store constraints with definitions for type narrowing
/// Definition IDs array for `definitions_by_use` and `public_definitions` to slice into.
all_definitions: Vec<Definition<'db>>,
/// Array of [`Definition`] in this scope.
all_definitions: IndexVec<ScopedDefinitionId, Definition<'db>>,
/// Definitions that can reach a [`ScopedUseId`].
definitions_by_use: IndexVec<ScopedUseId, Definitions>,
/// Array of constraints (as [`Expression`]) in this scope.
all_constraints: IndexVec<ScopedConstraintId, Expression<'db>>,
/// Definitions of each symbol visible at end of scope.
public_definitions: IndexVec<ScopedSymbolId, Definitions>,
/// [`SymbolState`] visible at a [`ScopedUseId`].
definitions_by_use: IndexVec<ScopedUseId, SymbolState>,
/// [`SymbolState`] visible at end of scope for each symbol.
public_definitions: IndexVec<ScopedSymbolId, SymbolState>,
}
impl<'db> UseDefMap<'db> {
pub(crate) fn use_definitions(&self, use_id: ScopedUseId) -> &[Definition<'db>] {
&self.all_definitions[self.definitions_by_use[use_id].definitions_range.clone()]
pub(crate) fn use_definitions(
&self,
use_id: ScopedUseId,
) -> DefinitionWithConstraintsIterator<'_, 'db> {
DefinitionWithConstraintsIterator {
all_definitions: &self.all_definitions,
all_constraints: &self.all_constraints,
inner: self.definitions_by_use[use_id].visible_definitions(),
}
}
pub(crate) fn use_may_be_unbound(&self, use_id: ScopedUseId) -> bool {
self.definitions_by_use[use_id].may_be_unbound
self.definitions_by_use[use_id].may_be_unbound()
}
pub(crate) fn public_definitions(&self, symbol: ScopedSymbolId) -> &[Definition<'db>] {
&self.all_definitions[self.public_definitions[symbol].definitions_range.clone()]
pub(crate) fn public_definitions(
&self,
symbol: ScopedSymbolId,
) -> DefinitionWithConstraintsIterator<'_, 'db> {
DefinitionWithConstraintsIterator {
all_definitions: &self.all_definitions,
all_constraints: &self.all_constraints,
inner: self.public_definitions[symbol].visible_definitions(),
}
}
pub(crate) fn public_may_be_unbound(&self, symbol: ScopedSymbolId) -> bool {
self.public_definitions[symbol].may_be_unbound
self.public_definitions[symbol].may_be_unbound()
}
}
/// Definitions visible for a symbol at a particular use (or end-of-scope).
#[derive(Clone, Debug, PartialEq, Eq)]
struct Definitions {
/// [`Range`] in `all_definitions` of the visible definition IDs.
definitions_range: Range<usize>,
/// Is the symbol possibly unbound at this point?
may_be_unbound: bool,
}
impl Definitions {
/// The default state of a symbol is "no definitions, may be unbound", aka definitely-unbound.
fn unbound() -> Self {
Self {
definitions_range: Range::default(),
may_be_unbound: true,
}
}
}
impl Default for Definitions {
fn default() -> Self {
Definitions::unbound()
}
}
/// A snapshot of the visible definitions for each symbol at a particular point in control flow.
#[derive(Clone, Debug)]
pub(super) struct FlowSnapshot {
definitions_by_symbol: IndexVec<ScopedSymbolId, Definitions>,
}
#[derive(Debug)]
pub(crate) struct DefinitionWithConstraintsIterator<'map, 'db> {
all_definitions: &'map IndexVec<ScopedDefinitionId, Definition<'db>>,
all_constraints: &'map IndexVec<ScopedConstraintId, Expression<'db>>,
inner: DefinitionIdWithConstraintsIterator<'map>,
}
impl<'map, 'db> Iterator for DefinitionWithConstraintsIterator<'map, 'db> {
type Item = DefinitionWithConstraints<'map, 'db>;
fn next(&mut self) -> Option<Self::Item> {
self.inner
.next()
.map(|def_id_with_constraints| DefinitionWithConstraints {
definition: self.all_definitions[def_id_with_constraints.definition],
constraints: ConstraintsIterator {
all_constraints: self.all_constraints,
constraint_ids: def_id_with_constraints.constraint_ids,
},
})
}
}
impl std::iter::FusedIterator for DefinitionWithConstraintsIterator<'_, '_> {}
pub(crate) struct DefinitionWithConstraints<'map, 'db> {
pub(crate) definition: Definition<'db>,
pub(crate) constraints: ConstraintsIterator<'map, 'db>,
}
pub(crate) struct ConstraintsIterator<'map, 'db> {
all_constraints: &'map IndexVec<ScopedConstraintId, Expression<'db>>,
constraint_ids: ConstraintIdIterator<'map>,
}
impl<'map, 'db> Iterator for ConstraintsIterator<'map, 'db> {
type Item = Expression<'db>;
fn next(&mut self) -> Option<Self::Item> {
self.constraint_ids
.next()
.map(|constraint_id| self.all_constraints[constraint_id])
}
}
impl std::iter::FusedIterator for ConstraintsIterator<'_, '_> {}
/// A snapshot of the definitions and constraints state at a particular point in control flow.
#[derive(Clone, Debug)]
pub(super) struct FlowSnapshot {
definitions_by_symbol: IndexVec<ScopedSymbolId, SymbolState>,
}
#[derive(Debug, Default)]
pub(super) struct UseDefMapBuilder<'db> {
/// Definition IDs array for `definitions_by_use` and `definitions_by_symbol` to slice into.
all_definitions: Vec<Definition<'db>>,
/// Append-only array of [`Definition`]; None is unbound.
all_definitions: IndexVec<ScopedDefinitionId, Definition<'db>>,
/// Append-only array of constraints (as [`Expression`]).
all_constraints: IndexVec<ScopedConstraintId, Expression<'db>>,
/// Visible definitions at each so-far-recorded use.
definitions_by_use: IndexVec<ScopedUseId, Definitions>,
definitions_by_use: IndexVec<ScopedUseId, SymbolState>,
/// Currently visible definitions for each symbol.
definitions_by_symbol: IndexVec<ScopedSymbolId, Definitions>,
definitions_by_symbol: IndexVec<ScopedSymbolId, SymbolState>,
}
impl<'db> UseDefMapBuilder<'db> {
pub(super) fn new() -> Self {
Self {
all_definitions: Vec::new(),
definitions_by_use: IndexVec::new(),
definitions_by_symbol: IndexVec::new(),
}
Self::default()
}
pub(super) fn add_symbol(&mut self, symbol: ScopedSymbolId) {
let new_symbol = self.definitions_by_symbol.push(Definitions::unbound());
let new_symbol = self.definitions_by_symbol.push(SymbolState::unbound());
debug_assert_eq!(symbol, new_symbol);
}
@@ -227,13 +286,15 @@ impl<'db> UseDefMapBuilder<'db> {
) {
// We have a new definition of a symbol; this replaces any previous definitions in this
// path.
let def_idx = self.all_definitions.len();
self.all_definitions.push(definition);
self.definitions_by_symbol[symbol] = Definitions {
#[allow(clippy::range_plus_one)]
definitions_range: def_idx..(def_idx + 1),
may_be_unbound: false,
};
let def_id = self.all_definitions.push(definition);
self.definitions_by_symbol[symbol] = SymbolState::with(def_id);
}
pub(super) fn record_constraint(&mut self, constraint: Expression<'db>) {
let constraint_id = self.all_constraints.push(constraint);
for definitions in &mut self.definitions_by_symbol {
definitions.add_constraint(constraint_id);
}
}
pub(super) fn record_use(&mut self, symbol: ScopedSymbolId, use_id: ScopedUseId) {
@@ -265,15 +326,15 @@ impl<'db> UseDefMapBuilder<'db> {
// If the snapshot we are restoring is missing some symbols we've recorded since, we need
// to fill them in so the symbol IDs continue to line up. Since they don't exist in the
// snapshot, the correct state to fill them in with is "unbound", the default.
// snapshot, the correct state to fill them in with is "unbound".
self.definitions_by_symbol
.resize(num_symbols, Definitions::unbound());
.resize(num_symbols, SymbolState::unbound());
}
/// Merge the given snapshot into the current state, reflecting that we might have taken either
/// path to get here. The new visible-definitions state for each symbol should include
/// definitions from both the prior state and the snapshot.
pub(super) fn merge(&mut self, snapshot: &FlowSnapshot) {
pub(super) fn merge(&mut self, snapshot: FlowSnapshot) {
// The tricky thing about merging two Ranges pointing into `all_definitions` is that if the
// two Ranges aren't already adjacent in `all_definitions`, we will have to copy at least
// one or the other of the ranges to the end of `all_definitions` so as to make them
@@ -287,66 +348,26 @@ impl<'db> UseDefMapBuilder<'db> {
// greater than the number of known symbols in a previously-taken snapshot.
debug_assert!(self.definitions_by_symbol.len() >= snapshot.definitions_by_symbol.len());
for (symbol_id, current) in self.definitions_by_symbol.iter_mut_enumerated() {
let Some(snapshot) = snapshot.definitions_by_symbol.get(symbol_id) else {
// Symbol not present in snapshot, so it's unbound from that path.
current.may_be_unbound = true;
continue;
};
// If the symbol can be unbound in either predecessor, it can be unbound post-merge.
current.may_be_unbound |= snapshot.may_be_unbound;
// Merge the definition ranges.
let current = &mut current.definitions_range;
let snapshot = &snapshot.definitions_range;
// We never create reversed ranges.
debug_assert!(current.end >= current.start);
debug_assert!(snapshot.end >= snapshot.start);
if current == snapshot {
// Ranges already identical, nothing to do.
} else if snapshot.is_empty() {
// Merging from an empty range; nothing to do.
} else if (*current).is_empty() {
// Merging to an empty range; just use the incoming range.
*current = snapshot.clone();
} else if snapshot.end >= current.start && snapshot.start <= current.end {
// Ranges are adjacent or overlapping, merge them in-place.
*current = current.start.min(snapshot.start)..current.end.max(snapshot.end);
} else if current.end == self.all_definitions.len() {
// Ranges are not adjacent or overlapping, `current` is at the end of
// `all_definitions`, we need to copy `snapshot` to the end so they are adjacent
// and can be merged into one range.
self.all_definitions.extend_from_within(snapshot.clone());
current.end = self.all_definitions.len();
} else if snapshot.end == self.all_definitions.len() {
// Ranges are not adjacent or overlapping, `snapshot` is at the end of
// `all_definitions`, we need to copy `current` to the end so they are adjacent and
// can be merged into one range.
self.all_definitions.extend_from_within(current.clone());
current.start = snapshot.start;
current.end = self.all_definitions.len();
let mut snapshot_definitions_iter = snapshot.definitions_by_symbol.into_iter();
for current in &mut self.definitions_by_symbol {
if let Some(snapshot) = snapshot_definitions_iter.next() {
current.merge(snapshot);
} else {
// Ranges are not adjacent and neither one is at the end of `all_definitions`, we
// have to copy both to the end so they are adjacent and we can merge them.
let start = self.all_definitions.len();
self.all_definitions.extend_from_within(current.clone());
self.all_definitions.extend_from_within(snapshot.clone());
current.start = start;
current.end = self.all_definitions.len();
// Symbol not present in snapshot, so it's unbound from that path.
current.add_unbound();
}
}
}
pub(super) fn finish(mut self) -> UseDefMap<'db> {
self.all_definitions.shrink_to_fit();
self.all_constraints.shrink_to_fit();
self.definitions_by_symbol.shrink_to_fit();
self.definitions_by_use.shrink_to_fit();
UseDefMap {
all_definitions: self.all_definitions,
all_constraints: self.all_constraints,
definitions_by_use: self.definitions_by_use,
public_definitions: self.definitions_by_symbol,
}

View File

@@ -0,0 +1,228 @@
/// Ordered set of `u32`.
///
/// Uses an inline bit-set for small values (up to 64 * B), falls back to heap allocated vector of
/// blocks for larger values.
#[derive(Debug, Clone, PartialEq, Eq)]
pub(super) enum BitSet<const B: usize> {
/// Bit-set (in 64-bit blocks) for the first 64 * B entries.
Inline([u64; B]),
/// Overflow beyond 64 * B.
Heap(Vec<u64>),
}
impl<const B: usize> Default for BitSet<B> {
fn default() -> Self {
// B * 64 must fit in a u32, or else we have unusable bits; this assertion makes the
// truncating casts to u32 below safe. This would be better as a const assertion, but
// that's not possible on stable with const generic params. (B should never really be
// anywhere close to this large.)
assert!(B * 64 < (u32::MAX as usize));
// This implementation requires usize >= 32 bits.
static_assertions::const_assert!(usize::BITS >= 32);
Self::Inline([0; B])
}
}
impl<const B: usize> BitSet<B> {
/// Create and return a new [`BitSet`] with a single `value` inserted.
pub(super) fn with(value: u32) -> Self {
let mut bitset = Self::default();
bitset.insert(value);
bitset
}
/// Convert from Inline to Heap, if needed, and resize the Heap vector, if needed.
fn resize(&mut self, value: u32) {
let num_blocks_needed = (value / 64) + 1;
match self {
Self::Inline(blocks) => {
let mut vec = blocks.to_vec();
vec.resize(num_blocks_needed as usize, 0);
*self = Self::Heap(vec);
}
Self::Heap(vec) => {
vec.resize(num_blocks_needed as usize, 0);
}
}
}
fn blocks_mut(&mut self) -> &mut [u64] {
match self {
Self::Inline(blocks) => blocks.as_mut_slice(),
Self::Heap(blocks) => blocks.as_mut_slice(),
}
}
fn blocks(&self) -> &[u64] {
match self {
Self::Inline(blocks) => blocks.as_slice(),
Self::Heap(blocks) => blocks.as_slice(),
}
}
/// Insert a value into the [`BitSet`].
///
/// Return true if the value was newly inserted, false if already present.
pub(super) fn insert(&mut self, value: u32) -> bool {
let value_usize = value as usize;
let (block, index) = (value_usize / 64, value_usize % 64);
if block >= self.blocks().len() {
self.resize(value);
}
let blocks = self.blocks_mut();
let missing = blocks[block] & (1 << index) == 0;
blocks[block] |= 1 << index;
missing
}
/// Intersect in-place with another [`BitSet`].
pub(super) fn intersect(&mut self, other: &BitSet<B>) {
let my_blocks = self.blocks_mut();
let other_blocks = other.blocks();
let min_len = my_blocks.len().min(other_blocks.len());
for i in 0..min_len {
my_blocks[i] &= other_blocks[i];
}
for block in my_blocks.iter_mut().skip(min_len) {
*block = 0;
}
}
/// Return an iterator over the values (in ascending order) in this [`BitSet`].
pub(super) fn iter(&self) -> BitSetIterator<'_, B> {
let blocks = self.blocks();
BitSetIterator {
blocks,
current_block_index: 0,
current_block: blocks[0],
}
}
}
/// Iterator over values in a [`BitSet`].
#[derive(Debug)]
pub(super) struct BitSetIterator<'a, const B: usize> {
/// The blocks we are iterating over.
blocks: &'a [u64],
/// The index of the block we are currently iterating through.
current_block_index: usize,
/// The block we are currently iterating through (and zeroing as we go.)
current_block: u64,
}
impl<const B: usize> Iterator for BitSetIterator<'_, B> {
type Item = u32;
fn next(&mut self) -> Option<Self::Item> {
while self.current_block == 0 {
if self.current_block_index + 1 >= self.blocks.len() {
return None;
}
self.current_block_index += 1;
self.current_block = self.blocks[self.current_block_index];
}
let lowest_bit_set = self.current_block.trailing_zeros();
// reset the lowest set bit, without a data dependency on `lowest_bit_set`
self.current_block &= self.current_block.wrapping_sub(1);
// SAFETY: `lowest_bit_set` cannot be more than 64, `current_block_index` cannot be more
// than `B - 1`, and we check above that `B * 64 < u32::MAX`. So both `64 *
// current_block_index` and the final value here must fit in u32.
#[allow(clippy::cast_possible_truncation)]
Some(lowest_bit_set + (64 * self.current_block_index) as u32)
}
}
impl<const B: usize> std::iter::FusedIterator for BitSetIterator<'_, B> {}
#[cfg(test)]
mod tests {
use super::BitSet;
fn assert_bitset<const B: usize>(bitset: &BitSet<B>, contents: &[u32]) {
assert_eq!(bitset.iter().collect::<Vec<_>>(), contents);
}
#[test]
fn iter() {
let mut b = BitSet::<1>::with(3);
b.insert(27);
b.insert(6);
assert!(matches!(b, BitSet::Inline(_)));
assert_bitset(&b, &[3, 6, 27]);
}
#[test]
fn iter_overflow() {
let mut b = BitSet::<1>::with(140);
b.insert(100);
b.insert(129);
assert!(matches!(b, BitSet::Heap(_)));
assert_bitset(&b, &[100, 129, 140]);
}
#[test]
fn intersect() {
let mut b1 = BitSet::<1>::with(4);
let mut b2 = BitSet::<1>::with(4);
b1.insert(23);
b2.insert(5);
b1.intersect(&b2);
assert_bitset(&b1, &[4]);
}
#[test]
fn intersect_mixed_1() {
let mut b1 = BitSet::<1>::with(4);
let mut b2 = BitSet::<1>::with(4);
b1.insert(89);
b2.insert(5);
b1.intersect(&b2);
assert_bitset(&b1, &[4]);
}
#[test]
fn intersect_mixed_2() {
let mut b1 = BitSet::<1>::with(4);
let mut b2 = BitSet::<1>::with(4);
b1.insert(23);
b2.insert(89);
b1.intersect(&b2);
assert_bitset(&b1, &[4]);
}
#[test]
fn intersect_heap() {
let mut b1 = BitSet::<1>::with(4);
let mut b2 = BitSet::<1>::with(4);
b1.insert(89);
b2.insert(90);
b1.intersect(&b2);
assert_bitset(&b1, &[4]);
}
#[test]
fn intersect_heap_2() {
let mut b1 = BitSet::<1>::with(89);
let mut b2 = BitSet::<1>::with(89);
b1.insert(91);
b2.insert(90);
b1.intersect(&b2);
assert_bitset(&b1, &[89]);
}
#[test]
fn multiple_blocks() {
let mut b = BitSet::<2>::with(120);
b.insert(45);
assert!(matches!(b, BitSet::Inline(_)));
assert_bitset(&b, &[45, 120]);
}
}

View File

@@ -0,0 +1,374 @@
//! Track visible definitions of a symbol, and applicable constraints per definition.
//!
//! These data structures operate entirely on scope-local newtype-indices for definitions and
//! constraints, referring to their location in the `all_definitions` and `all_constraints`
//! indexvecs in [`super::UseDefMapBuilder`].
//!
//! We need to track arbitrary associations between definitions and constraints, not just a single
//! set of currently dominating constraints (where "dominating" means "control flow must have
//! passed through it to reach this point"), because we can have dominating constraints that apply
//! to some definitions but not others, as in this code:
//!
//! ```python
//! x = 1 if flag else None
//! if x is not None:
//! if flag2:
//! x = 2 if flag else None
//! x
//! ```
//!
//! The `x is not None` constraint dominates the final use of `x`, but it applies only to the first
//! definition of `x`, not the second, so `None` is a possible value for `x`.
//!
//! And we can't just track, for each definition, an index into a list of dominating constraints,
//! either, because we can have definitions which are still visible, but subject to constraints
//! that are no longer dominating, as in this code:
//!
//! ```python
//! x = 0
//! if flag1:
//! x = 1 if flag2 else None
//! assert x is not None
//! x
//! ```
//!
//! From the point of view of the final use of `x`, the `x is not None` constraint no longer
//! dominates, but it does dominate the `x = 1 if flag2 else None` definition, so we have to keep
//! track of that.
//!
//! The data structures used here ([`BitSet`] and [`smallvec::SmallVec`]) optimize for keeping all
//! data inline (avoiding lots of scattered allocations) in small-to-medium cases, and falling back
//! to heap allocation to be able to scale to arbitrary numbers of definitions and constraints when
//! needed.
use super::bitset::{BitSet, BitSetIterator};
use ruff_index::newtype_index;
use smallvec::SmallVec;
/// A newtype-index for a definition in a particular scope.
#[newtype_index]
pub(super) struct ScopedDefinitionId;
/// A newtype-index for a constraint expression in a particular scope.
#[newtype_index]
pub(super) struct ScopedConstraintId;
/// Can reference this * 64 total definitions inline; more will fall back to the heap.
const INLINE_DEFINITION_BLOCKS: usize = 3;
/// A [`BitSet`] of [`ScopedDefinitionId`], representing visible definitions of a symbol in a scope.
type Definitions = BitSet<INLINE_DEFINITION_BLOCKS>;
type DefinitionsIterator<'a> = BitSetIterator<'a, INLINE_DEFINITION_BLOCKS>;
/// Can reference this * 64 total constraints inline; more will fall back to the heap.
const INLINE_CONSTRAINT_BLOCKS: usize = 2;
/// Can keep inline this many visible definitions per symbol at a given time; more will go to heap.
const INLINE_VISIBLE_DEFINITIONS_PER_SYMBOL: usize = 4;
/// One [`BitSet`] of applicable [`ScopedConstraintId`] per visible definition.
type InlineConstraintArray =
[BitSet<INLINE_CONSTRAINT_BLOCKS>; INLINE_VISIBLE_DEFINITIONS_PER_SYMBOL];
type Constraints = SmallVec<InlineConstraintArray>;
type ConstraintsIterator<'a> = std::slice::Iter<'a, BitSet<INLINE_CONSTRAINT_BLOCKS>>;
type ConstraintsIntoIterator = smallvec::IntoIter<InlineConstraintArray>;
/// Visible definitions and narrowing constraints for a single symbol at some point in control flow.
#[derive(Clone, Debug, PartialEq, Eq)]
pub(super) struct SymbolState {
/// [`BitSet`]: which [`ScopedDefinitionId`] are visible for this symbol?
visible_definitions: Definitions,
/// For each definition, which [`ScopedConstraintId`] apply?
///
/// This is a [`smallvec::SmallVec`] which should always have one [`BitSet`] of constraints per
/// definition in `visible_definitions`.
constraints: Constraints,
/// Could the symbol be unbound at this point?
may_be_unbound: bool,
}
/// A single [`ScopedDefinitionId`] with an iterator of its applicable [`ScopedConstraintId`].
#[derive(Debug)]
pub(super) struct DefinitionIdWithConstraints<'a> {
pub(super) definition: ScopedDefinitionId,
pub(super) constraint_ids: ConstraintIdIterator<'a>,
}
impl SymbolState {
/// Return a new [`SymbolState`] representing an unbound symbol.
pub(super) fn unbound() -> Self {
Self {
visible_definitions: Definitions::default(),
constraints: Constraints::default(),
may_be_unbound: true,
}
}
/// Return a new [`SymbolState`] representing a symbol with a single visible definition.
pub(super) fn with(definition_id: ScopedDefinitionId) -> Self {
let mut constraints = Constraints::with_capacity(1);
constraints.push(BitSet::default());
Self {
visible_definitions: Definitions::with(definition_id.into()),
constraints,
may_be_unbound: false,
}
}
/// Add Unbound as a possibility for this symbol.
pub(super) fn add_unbound(&mut self) {
self.may_be_unbound = true;
}
/// Add given constraint to all currently-visible definitions.
pub(super) fn add_constraint(&mut self, constraint_id: ScopedConstraintId) {
for bitset in &mut self.constraints {
bitset.insert(constraint_id.into());
}
}
/// Merge another [`SymbolState`] into this one.
pub(super) fn merge(&mut self, b: SymbolState) {
let mut a = Self {
visible_definitions: Definitions::default(),
constraints: Constraints::default(),
may_be_unbound: self.may_be_unbound || b.may_be_unbound,
};
std::mem::swap(&mut a, self);
let mut a_defs_iter = a.visible_definitions.iter();
let mut b_defs_iter = b.visible_definitions.iter();
let mut a_constraints_iter = a.constraints.into_iter();
let mut b_constraints_iter = b.constraints.into_iter();
let mut opt_a_def: Option<u32> = a_defs_iter.next();
let mut opt_b_def: Option<u32> = b_defs_iter.next();
// Iterate through the definitions from `a` and `b`, always processing the lower definition
// ID first, and pushing each definition onto the merged `SymbolState` with its
// constraints. If a definition is found in both `a` and `b`, push it with the intersection
// of the constraints from the two paths; a constraint that applies from only one possible
// path is irrelevant.
// Helper to push `def`, with constraints in `constraints_iter`, onto `self`.
let push = |def, constraints_iter: &mut ConstraintsIntoIterator, merged: &mut Self| {
merged.visible_definitions.insert(def);
// SAFETY: we only ever create SymbolState with either no definitions and no constraint
// bitsets (`::unbound`) or one definition and one constraint bitset (`::with`), and
// `::merge` always pushes one definition and one constraint bitset together (just
// below), so the number of definitions and the number of constraint bitsets can never
// get out of sync.
let constraints = constraints_iter
.next()
.expect("definitions and constraints length mismatch");
merged.constraints.push(constraints);
};
loop {
match (opt_a_def, opt_b_def) {
(Some(a_def), Some(b_def)) => match a_def.cmp(&b_def) {
std::cmp::Ordering::Less => {
// Next definition ID is only in `a`, push it to `self` and advance `a`.
push(a_def, &mut a_constraints_iter, self);
opt_a_def = a_defs_iter.next();
}
std::cmp::Ordering::Greater => {
// Next definition ID is only in `b`, push it to `self` and advance `b`.
push(b_def, &mut b_constraints_iter, self);
opt_b_def = b_defs_iter.next();
}
std::cmp::Ordering::Equal => {
// Next definition is in both; push to `self` and intersect constraints.
push(a_def, &mut b_constraints_iter, self);
// SAFETY: we only ever create SymbolState with either no definitions and
// no constraint bitsets (`::unbound`) or one definition and one constraint
// bitset (`::with`), and `::merge` always pushes one definition and one
// constraint bitset together (just below), so the number of definitions
// and the number of constraint bitsets can never get out of sync.
let a_constraints = a_constraints_iter
.next()
.expect("definitions and constraints length mismatch");
// If the same definition is visible through both paths, any constraint
// that applies on only one path is irrelevant to the resulting type from
// unioning the two paths, so we intersect the constraints.
self.constraints
.last_mut()
.unwrap()
.intersect(&a_constraints);
opt_a_def = a_defs_iter.next();
opt_b_def = b_defs_iter.next();
}
},
(Some(a_def), None) => {
// We've exhausted `b`, just push the def from `a` and move on to the next.
push(a_def, &mut a_constraints_iter, self);
opt_a_def = a_defs_iter.next();
}
(None, Some(b_def)) => {
// We've exhausted `a`, just push the def from `b` and move on to the next.
push(b_def, &mut b_constraints_iter, self);
opt_b_def = b_defs_iter.next();
}
(None, None) => break,
}
}
}
/// Get iterator over visible definitions with constraints.
pub(super) fn visible_definitions(&self) -> DefinitionIdWithConstraintsIterator {
DefinitionIdWithConstraintsIterator {
definitions: self.visible_definitions.iter(),
constraints: self.constraints.iter(),
}
}
/// Could the symbol be unbound?
pub(super) fn may_be_unbound(&self) -> bool {
self.may_be_unbound
}
}
/// The default state of a symbol (if we've seen no definitions of it) is unbound.
impl Default for SymbolState {
fn default() -> Self {
SymbolState::unbound()
}
}
#[derive(Debug)]
pub(super) struct DefinitionIdWithConstraintsIterator<'a> {
definitions: DefinitionsIterator<'a>,
constraints: ConstraintsIterator<'a>,
}
impl<'a> Iterator for DefinitionIdWithConstraintsIterator<'a> {
type Item = DefinitionIdWithConstraints<'a>;
fn next(&mut self) -> Option<Self::Item> {
match (self.definitions.next(), self.constraints.next()) {
(None, None) => None,
(Some(def), Some(constraints)) => Some(DefinitionIdWithConstraints {
definition: ScopedDefinitionId::from_u32(def),
constraint_ids: ConstraintIdIterator {
wrapped: constraints.iter(),
},
}),
// SAFETY: see above.
_ => unreachable!("definitions and constraints length mismatch"),
}
}
}
impl std::iter::FusedIterator for DefinitionIdWithConstraintsIterator<'_> {}
#[derive(Debug)]
pub(super) struct ConstraintIdIterator<'a> {
wrapped: BitSetIterator<'a, INLINE_CONSTRAINT_BLOCKS>,
}
impl Iterator for ConstraintIdIterator<'_> {
type Item = ScopedConstraintId;
fn next(&mut self) -> Option<Self::Item> {
self.wrapped.next().map(ScopedConstraintId::from_u32)
}
}
impl std::iter::FusedIterator for ConstraintIdIterator<'_> {}
#[cfg(test)]
mod tests {
use super::{ScopedConstraintId, ScopedDefinitionId, SymbolState};
impl SymbolState {
pub(crate) fn assert(&self, may_be_unbound: bool, expected: &[&str]) {
assert_eq!(self.may_be_unbound(), may_be_unbound);
let actual = self
.visible_definitions()
.map(|def_id_with_constraints| {
format!(
"{}<{}>",
def_id_with_constraints.definition.as_u32(),
def_id_with_constraints
.constraint_ids
.map(ScopedConstraintId::as_u32)
.map(|idx| idx.to_string())
.collect::<Vec<_>>()
.join(", ")
)
})
.collect::<Vec<_>>();
assert_eq!(actual, expected);
}
}
#[test]
fn unbound() {
let cd = SymbolState::unbound();
cd.assert(true, &[]);
}
#[test]
fn with() {
let cd = SymbolState::with(ScopedDefinitionId::from_u32(0));
cd.assert(false, &["0<>"]);
}
#[test]
fn add_unbound() {
let mut cd = SymbolState::with(ScopedDefinitionId::from_u32(0));
cd.add_unbound();
cd.assert(true, &["0<>"]);
}
#[test]
fn add_constraint() {
let mut cd = SymbolState::with(ScopedDefinitionId::from_u32(0));
cd.add_constraint(ScopedConstraintId::from_u32(0));
cd.assert(false, &["0<0>"]);
}
#[test]
fn merge() {
// merging the same definition with the same constraint keeps the constraint
let mut cd0a = SymbolState::with(ScopedDefinitionId::from_u32(0));
cd0a.add_constraint(ScopedConstraintId::from_u32(0));
let mut cd0b = SymbolState::with(ScopedDefinitionId::from_u32(0));
cd0b.add_constraint(ScopedConstraintId::from_u32(0));
cd0a.merge(cd0b);
let mut cd0 = cd0a;
cd0.assert(false, &["0<0>"]);
// merging the same definition with differing constraints drops all constraints
let mut cd1a = SymbolState::with(ScopedDefinitionId::from_u32(1));
cd1a.add_constraint(ScopedConstraintId::from_u32(1));
let mut cd1b = SymbolState::with(ScopedDefinitionId::from_u32(1));
cd1b.add_constraint(ScopedConstraintId::from_u32(2));
cd1a.merge(cd1b);
let cd1 = cd1a;
cd1.assert(false, &["1<>"]);
// merging a constrained definition with unbound keeps both
let mut cd2a = SymbolState::with(ScopedDefinitionId::from_u32(2));
cd2a.add_constraint(ScopedConstraintId::from_u32(3));
let cd2b = SymbolState::unbound();
cd2a.merge(cd2b);
let cd2 = cd2a;
cd2.assert(true, &["2<3>"]);
// merging different definitions keeps them each with their existing constraints
cd0.merge(cd2);
let cd = cd0;
cd.assert(true, &["0<0>", "2<3>"]);
}
}

View File

@@ -1,7 +1,7 @@
use ruff_db::files::{File, FilePath};
use ruff_db::source::line_index;
use ruff_python_ast as ast;
use ruff_python_ast::{Expr, ExpressionRef, StmtClassDef};
use ruff_python_ast::{Expr, ExpressionRef};
use ruff_source_file::LineIndex;
use crate::module_name::ModuleName;
@@ -147,29 +147,24 @@ impl HasTy for ast::Expr {
}
}
impl HasTy for ast::StmtFunctionDef {
fn ty<'db>(&self, model: &SemanticModel<'db>) -> Type<'db> {
let index = semantic_index(model.db, model.file);
let definition = index.definition(self);
definition_ty(model.db, definition)
}
macro_rules! impl_definition_has_ty {
($ty: ty) => {
impl HasTy for $ty {
#[inline]
fn ty<'db>(&self, model: &SemanticModel<'db>) -> Type<'db> {
let index = semantic_index(model.db, model.file);
let definition = index.definition(self);
definition_ty(model.db, definition)
}
}
};
}
impl HasTy for StmtClassDef {
fn ty<'db>(&self, model: &SemanticModel<'db>) -> Type<'db> {
let index = semantic_index(model.db, model.file);
let definition = index.definition(self);
definition_ty(model.db, definition)
}
}
impl HasTy for ast::Alias {
fn ty<'db>(&self, model: &SemanticModel<'db>) -> Type<'db> {
let index = semantic_index(model.db, model.file);
let definition = index.definition(self);
definition_ty(model.db, definition)
}
}
impl_definition_has_ty!(ast::StmtFunctionDef);
impl_definition_has_ty!(ast::StmtClassDef);
impl_definition_has_ty!(ast::Alias);
impl_definition_has_ty!(ast::Parameter);
impl_definition_has_ty!(ast::ParameterWithDefault);
#[cfg(test)]
mod tests {

View File

@@ -4,15 +4,22 @@ use ruff_python_ast::name::Name;
use crate::builtins::builtins_scope;
use crate::semantic_index::definition::Definition;
use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId};
use crate::semantic_index::{global_scope, symbol_table, use_def_map};
use crate::semantic_index::{
global_scope, symbol_table, use_def_map, DefinitionWithConstraints,
DefinitionWithConstraintsIterator,
};
use crate::types::narrow::narrowing_constraint;
use crate::{Db, FxOrderSet};
mod builder;
mod display;
mod infer;
mod narrow;
pub(crate) use self::builder::UnionBuilder;
pub(crate) use self::infer::{infer_definition_types, infer_scope_types};
pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder};
pub(crate) use self::infer::{
infer_definition_types, infer_expression_types, infer_scope_types, TypeInference,
};
/// Infer the public type of a symbol (its type as seen from outside its scope).
pub(crate) fn symbol_ty<'db>(
@@ -82,10 +89,31 @@ pub(crate) fn definition_ty<'db>(db: &'db dyn Db, definition: Definition<'db>) -
/// provide an `unbound_ty`.
pub(crate) fn definitions_ty<'db>(
db: &'db dyn Db,
definitions: &[Definition<'db>],
definitions_with_constraints: DefinitionWithConstraintsIterator<'_, 'db>,
unbound_ty: Option<Type<'db>>,
) -> Type<'db> {
let def_types = definitions.iter().map(|def| definition_ty(db, *def));
let def_types = definitions_with_constraints.map(
|DefinitionWithConstraints {
definition,
constraints,
}| {
let mut constraint_tys =
constraints.filter_map(|test| narrowing_constraint(db, test, definition));
let definition_ty = definition_ty(db, definition);
if let Some(first_constraint_ty) = constraint_tys.next() {
let mut builder = IntersectionBuilder::new(db);
builder = builder
.add_positive(definition_ty)
.add_positive(first_constraint_ty);
for constraint_ty in constraint_tys {
builder = builder.add_positive(constraint_ty);
}
builder.build()
} else {
definition_ty
}
},
);
let mut all_types = unbound_ty.into_iter().chain(def_types);
let Some(first) = all_types.next() else {
@@ -113,7 +141,7 @@ pub enum Type<'db> {
Any,
/// the empty set of values
Never,
/// unknown type (no annotation)
/// unknown type (either no annotation, or some kind of type error)
/// equivalent to Any, or possibly to object in strict mode
Unknown,
/// name does not exist or is not bound to any value (this represents an error, but with some
@@ -149,6 +177,35 @@ impl<'db> Type<'db> {
matches!(self, Type::Unknown)
}
pub const fn is_never(&self) -> bool {
matches!(self, Type::Never)
}
pub fn may_be_unbound(&self, db: &'db dyn Db) -> bool {
match self {
Type::Unbound => true,
Type::Union(union) => union.contains(db, Type::Unbound),
// Unbound can't appear in an intersection, because an intersection with Unbound
// simplifies to just Unbound.
_ => false,
}
}
#[must_use]
pub fn replace_unbound_with(&self, db: &'db dyn Db, replacement: Type<'db>) -> Type<'db> {
match self {
Type::Unbound => replacement,
Type::Union(union) => union
.elements(db)
.into_iter()
.fold(UnionBuilder::new(db), |builder, ty| {
builder.add(ty.replace_unbound_with(db, replacement))
})
.build(),
ty => *ty,
}
}
#[must_use]
pub fn member(&self, db: &'db dyn Db, name: &Name) -> Type<'db> {
match self {

View File

@@ -65,7 +65,6 @@ impl<'db> UnionBuilder<'db> {
}
}
#[allow(unused)]
#[derive(Clone)]
pub(crate) struct IntersectionBuilder<'db> {
// Really this builds a union-of-intersections, because we always keep our set-theoretic types
@@ -78,8 +77,7 @@ pub(crate) struct IntersectionBuilder<'db> {
}
impl<'db> IntersectionBuilder<'db> {
#[allow(dead_code)]
fn new(db: &'db dyn Db) -> Self {
pub(crate) fn new(db: &'db dyn Db) -> Self {
Self {
db,
intersections: vec![InnerIntersectionBuilder::new()],
@@ -93,8 +91,7 @@ impl<'db> IntersectionBuilder<'db> {
}
}
#[allow(dead_code)]
fn add_positive(mut self, ty: Type<'db>) -> Self {
pub(crate) fn add_positive(mut self, ty: Type<'db>) -> Self {
if let Type::Union(union) = ty {
// Distribute ourself over this union: for each union element, clone ourself and
// intersect with that union element, then create a new union-of-intersections with all
@@ -122,8 +119,7 @@ impl<'db> IntersectionBuilder<'db> {
}
}
#[allow(dead_code)]
fn add_negative(mut self, ty: Type<'db>) -> Self {
pub(crate) fn add_negative(mut self, ty: Type<'db>) -> Self {
// See comments above in `add_positive`; this is just the negated version.
if let Type::Union(union) = ty {
union
@@ -142,8 +138,7 @@ impl<'db> IntersectionBuilder<'db> {
}
}
#[allow(dead_code)]
fn build(mut self) -> Type<'db> {
pub(crate) fn build(mut self) -> Type<'db> {
// Avoid allocating the UnionBuilder unnecessarily if we have just one intersection:
if self.intersections.len() == 1 {
self.intersections.pop().unwrap().build(self.db)
@@ -157,7 +152,6 @@ impl<'db> IntersectionBuilder<'db> {
}
}
#[allow(unused)]
#[derive(Debug, Clone, Default)]
struct InnerIntersectionBuilder<'db> {
positive: FxOrderSet<Type<'db>>,
@@ -201,6 +195,7 @@ impl<'db> InnerIntersectionBuilder<'db> {
self.negative.retain(|elem| !pos.contains(elem));
}
Type::Never => {}
Type::Unbound => {}
_ => {
if !self.positive.remove(&ty) {
self.negative.insert(ty);
@@ -214,9 +209,23 @@ impl<'db> InnerIntersectionBuilder<'db> {
// Never is a subtype of all types
if self.positive.contains(&Type::Never) {
self.positive.clear();
self.positive.retain(Type::is_never);
self.negative.clear();
self.positive.insert(Type::Never);
}
if self.positive.contains(&Type::Unbound) {
self.positive.retain(Type::is_unbound);
self.negative.clear();
}
// None intersects only with object
for pos in &self.positive {
if let Type::Instance(_) = pos {
// could be `object` type
} else {
self.negative.remove(&Type::None);
break;
}
}
}
@@ -426,4 +435,37 @@ mod tests {
assert_eq!(ty, Type::Never);
}
#[test]
fn build_intersection_simplify_positive_unbound() {
let db = setup_db();
let ty = IntersectionBuilder::new(&db)
.add_positive(Type::Unbound)
.add_positive(Type::IntLiteral(1))
.build();
assert_eq!(ty, Type::Unbound);
}
#[test]
fn build_intersection_simplify_negative_unbound() {
let db = setup_db();
let ty = IntersectionBuilder::new(&db)
.add_negative(Type::Unbound)
.add_positive(Type::IntLiteral(1))
.build();
assert_eq!(ty, Type::IntLiteral(1));
}
#[test]
fn build_intersection_simplify_negative_none() {
let db = setup_db();
let ty = IntersectionBuilder::new(&db)
.add_negative(Type::None)
.add_positive(Type::IntLiteral(1))
.build();
assert_eq!(ty, Type::IntLiteral(1));
}
}

View File

@@ -20,6 +20,8 @@
//!
//! Inferring types at any of the three region granularities returns a [`TypeInference`], which
//! holds types for every [`Definition`] and expression within the inferred region.
use std::num::NonZeroU32;
use rustc_hash::FxHashMap;
use salsa;
use salsa::plumbing::AsId;
@@ -27,11 +29,11 @@ use salsa::plumbing::AsId;
use ruff_db::files::File;
use ruff_db::parsed::parsed_module;
use ruff_python_ast as ast;
use ruff_python_ast::{ExprContext, TypeParams};
use ruff_python_ast::{Expr, ExprContext};
use crate::builtins::builtins_scope;
use crate::module_name::ModuleName;
use crate::module_resolver::resolve_module;
use crate::module_resolver::{file_to_module, resolve_module};
use crate::semantic_index::ast_ids::{HasScopedAstId, HasScopedUseId, ScopedExpressionId};
use crate::semantic_index::definition::{Definition, DefinitionKind, DefinitionNodeKey};
use crate::semantic_index::expression::Expression;
@@ -292,7 +294,11 @@ impl<'db> TypeInferenceBuilder<'db> {
);
}
DefinitionKind::Assignment(assignment) => {
self.infer_assignment_definition(assignment.assignment(), definition);
self.infer_assignment_definition(
assignment.target(),
assignment.assignment(),
definition,
);
}
DefinitionKind::AnnotatedAssignment(annotated_assignment) => {
self.infer_annotated_assignment_definition(annotated_assignment.node(), definition);
@@ -307,11 +313,17 @@ impl<'db> TypeInferenceBuilder<'db> {
definition,
);
}
DefinitionKind::Parameter(parameter) => {
self.infer_parameter_definition(parameter, definition);
}
DefinitionKind::ParameterWithDefault(parameter_with_default) => {
self.infer_parameter_with_default_definition(parameter_with_default, definition);
}
}
}
fn infer_region_expression(&mut self, expression: Expression<'db>) {
self.infer_expression(expression.node(self.db));
self.infer_expression(expression.node_ref(self.db));
}
fn infer_module(&mut self, module: &ast::ModModule) {
@@ -421,6 +433,13 @@ impl<'db> TypeInferenceBuilder<'db> {
.map(|decorator| self.infer_decorator(decorator))
.collect();
for default in parameters
.iter_non_variadic_params()
.filter_map(|param| param.default.as_deref())
{
self.infer_expression(default);
}
// If there are type params, parameters and returns are evaluated in that scope.
if type_params.is_none() {
self.infer_parameters(parameters);
@@ -458,10 +477,12 @@ impl<'db> TypeInferenceBuilder<'db> {
let ast::ParameterWithDefault {
range: _,
parameter,
default,
default: _,
} = parameter_with_default;
self.infer_parameter(parameter);
self.infer_optional_expression(default.as_deref());
self.infer_optional_expression(parameter.annotation.as_deref());
self.infer_definition(parameter_with_default);
}
fn infer_parameter(&mut self, parameter: &ast::Parameter) {
@@ -470,7 +491,29 @@ impl<'db> TypeInferenceBuilder<'db> {
name: _,
annotation,
} = parameter;
self.infer_optional_expression(annotation.as_deref());
self.infer_definition(parameter);
}
fn infer_parameter_with_default_definition(
&mut self,
_parameter_with_default: &ast::ParameterWithDefault,
definition: Definition<'db>,
) {
// TODO(dhruvmanila): Infer types from annotation or default expression
self.types.definitions.insert(definition, Type::Unknown);
}
fn infer_parameter_definition(
&mut self,
_parameter: &ast::Parameter,
definition: Definition<'db>,
) {
// TODO(dhruvmanila): Annotation expression is resolved at the enclosing scope, infer the
// parameter type from there
self.types.definitions.insert(definition, Type::Unknown);
}
fn infer_class_definition_statement(&mut self, class: &ast::StmtClassDef) {
@@ -667,6 +710,7 @@ impl<'db> TypeInferenceBuilder<'db> {
fn infer_assignment_definition(
&mut self,
target: &ast::ExprName,
assignment: &ast::StmtAssign,
definition: Definition<'db>,
) {
@@ -676,6 +720,9 @@ impl<'db> TypeInferenceBuilder<'db> {
let value_ty = self
.types
.expression_ty(assignment.value.scoped_ast_id(self.db, self.scope));
self.types
.expressions
.insert(target.scoped_ast_id(self.db, self.scope), value_ty);
self.types.definitions.insert(definition, value_ty);
}
@@ -785,7 +832,7 @@ impl<'db> TypeInferenceBuilder<'db> {
asname: _,
} = alias;
let module_ty = self.module_ty_from_name(name);
let module_ty = self.module_ty_from_name(ModuleName::new(name));
self.types.definitions.insert(definition, module_ty);
}
@@ -823,27 +870,82 @@ impl<'db> TypeInferenceBuilder<'db> {
self.infer_optional_expression(cause.as_deref());
}
/// Given a `from .foo import bar` relative import, resolve the relative module
/// we're importing `bar` from into an absolute [`ModuleName`]
/// using the name of the module we're currently analyzing.
///
/// - `level` is the number of dots at the beginning of the relative module name:
/// - `from .foo.bar import baz` => `level == 1`
/// - `from ...foo.bar import baz` => `level == 3`
/// - `tail` is the relative module name stripped of all leading dots:
/// - `from .foo import bar` => `tail == "foo"`
/// - `from ..foo.bar import baz` => `tail == "foo.bar"`
fn relative_module_name(&self, tail: Option<&str>, level: NonZeroU32) -> Option<ModuleName> {
let Some(module) = file_to_module(self.db, self.file) else {
tracing::debug!("Failed to resolve file {:?} to a module", self.file);
return None;
};
let mut level = level.get();
if module.kind().is_package() {
level -= 1;
}
let mut module_name = module.name().to_owned();
for _ in 0..level {
module_name = module_name.parent()?;
}
if let Some(tail) = tail {
if let Some(valid_tail) = ModuleName::new(tail) {
module_name.extend(&valid_tail);
} else {
tracing::debug!("Failed to resolve relative import due to invalid syntax");
return None;
}
}
Some(module_name)
}
fn infer_import_from_definition(
&mut self,
import_from: &ast::StmtImportFrom,
alias: &ast::Alias,
definition: Definition<'db>,
) {
let ast::StmtImportFrom { module, .. } = import_from;
let module_ty = if let Some(module) = module {
self.module_ty_from_name(module)
// TODO:
// - Absolute `*` imports (`from collections import *`)
// - Relative `*` imports (`from ...foo import *`)
// - Submodule imports (`from collections import abc`,
// where `abc` is a submodule of the `collections` package)
//
// For the last item, see the currently skipped tests
// `follow_relative_import_bare_to_module()` and
// `follow_nonexistent_import_bare_to_module()`.
let ast::StmtImportFrom { module, level, .. } = import_from;
tracing::trace!("Resolving imported object {alias:?} from statement {import_from:?}");
let module_name = if let Some(level) = NonZeroU32::new(*level) {
self.relative_module_name(module.as_deref(), level)
} else {
// TODO support relative imports
Type::Unknown
let module_name = module
.as_ref()
.expect("Non-relative import should always have a non-None `module`!");
ModuleName::new(module_name)
};
let module_ty = self.module_ty_from_name(module_name);
let ast::Alias {
range: _,
name,
asname: _,
} = alias;
let ty = module_ty.member(self.db, &Name::new(&name.id));
// If a symbol is unbound in the module the symbol was originally defined in,
// when we're trying to import the symbol from that module into "our" module,
// the runtime error will occur immediately (rather than when the symbol is *used*,
// as would be the case for a symbol with type `Unbound`), so it's appropriate to
// think of the type of the imported symbol as `Unknown` rather than `Unbound`
let ty = module_ty
.member(self.db, &Name::new(&name.id))
.replace_unbound_with(self.db, Type::Unknown);
self.types.definitions.insert(definition, ty);
}
@@ -859,11 +961,10 @@ impl<'db> TypeInferenceBuilder<'db> {
}
}
fn module_ty_from_name(&self, name: &ast::Identifier) -> Type<'db> {
let module = ModuleName::new(&name.id).and_then(|name| resolve_module(self.db, name));
module
.map(|module| Type::Module(module.file()))
.unwrap_or(Type::Unbound)
fn module_ty_from_name(&self, module_name: Option<ModuleName>) -> Type<'db> {
module_name
.and_then(|module_name| resolve_module(self.db, module_name))
.map_or(Type::Unknown, |module| Type::Module(module.file()))
}
fn infer_decorator(&mut self, decorator: &ast::Decorator) -> Type<'db> {
@@ -906,6 +1007,9 @@ impl<'db> TypeInferenceBuilder<'db> {
ast::Expr::NumberLiteral(literal) => self.infer_number_literal_expression(literal),
ast::Expr::BooleanLiteral(literal) => self.infer_boolean_literal_expression(literal),
ast::Expr::StringLiteral(literal) => self.infer_string_literal_expression(literal),
ast::Expr::BytesLiteral(bytes_literal) => {
self.infer_bytes_literal_expression(bytes_literal)
}
ast::Expr::FString(fstring) => self.infer_fstring_expression(fstring),
ast::Expr::EllipsisLiteral(literal) => self.infer_ellipsis_literal_expression(literal),
ast::Expr::Tuple(tuple) => self.infer_tuple_expression(tuple),
@@ -932,8 +1036,7 @@ impl<'db> TypeInferenceBuilder<'db> {
ast::Expr::Yield(yield_expression) => self.infer_yield_expression(yield_expression),
ast::Expr::YieldFrom(yield_from) => self.infer_yield_from_expression(yield_from),
ast::Expr::Await(await_expression) => self.infer_await_expression(await_expression),
_ => todo!("expression type resolution for {:?}", expression),
Expr::IpyEscapeCommand(_) => todo!("Implement Ipy escape command support"),
};
let expr_id = expression.scoped_ast_id(self.db, self.scope);
@@ -970,6 +1073,12 @@ impl<'db> TypeInferenceBuilder<'db> {
Type::Unknown
}
#[allow(clippy::unused_self)]
fn infer_bytes_literal_expression(&mut self, _literal: &ast::ExprBytesLiteral) -> Type<'db> {
// TODO
Type::Unknown
}
fn infer_fstring_expression(&mut self, fstring: &ast::ExprFString) -> Type<'db> {
let ast::ExprFString { range: _, value } = fstring;
@@ -1277,6 +1386,13 @@ impl<'db> TypeInferenceBuilder<'db> {
} = lambda_expression;
if let Some(parameters) = parameters {
for default in parameters
.iter_non_variadic_params()
.filter_map(|param| param.default.as_deref())
{
self.infer_expression(default);
}
self.infer_parameters(parameters);
}
@@ -1354,18 +1470,22 @@ impl<'db> TypeInferenceBuilder<'db> {
let symbol = symbols.symbol_by_name(id).unwrap();
if !symbol.is_defined() || !self.scope.is_function_like(self.db) {
// implicit global
let mut unbound_ty = if file_scope_id == FileScopeId::global() {
let unbound_ty = if file_scope_id == FileScopeId::global() {
Type::Unbound
} else {
global_symbol_ty_by_name(self.db, self.file, id)
};
// fallback to builtins
if matches!(unbound_ty, Type::Unbound)
if unbound_ty.may_be_unbound(self.db)
&& Some(self.scope) != builtins_scope(self.db)
{
unbound_ty = builtins_symbol_ty_by_name(self.db, id);
Some(unbound_ty.replace_unbound_with(
self.db,
builtins_symbol_ty_by_name(self.db, id),
))
} else {
Some(unbound_ty)
}
Some(unbound_ty)
} else {
Some(Type::Unbound)
}
@@ -1526,7 +1646,7 @@ impl<'db> TypeInferenceBuilder<'db> {
Type::Unknown
}
fn infer_type_parameters(&mut self, type_parameters: &TypeParams) {
fn infer_type_parameters(&mut self, type_parameters: &ast::TypeParams) {
let ast::TypeParams {
range: _,
type_params,
@@ -1573,6 +1693,7 @@ impl<'db> TypeInferenceBuilder<'db> {
#[cfg(test)]
mod tests {
use anyhow::Context;
use ruff_db::files::{system_path_to_file, File};
use ruff_db::parsed::parsed_module;
use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
@@ -1662,6 +1783,166 @@ mod tests {
Ok(())
}
#[test]
fn follow_relative_import_simple() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", ""),
("src/package/foo.py", "X = 42"),
("src/package/bar.py", "from .foo import X"),
])?;
assert_public_ty(&db, "src/package/bar.py", "X", "Literal[42]");
Ok(())
}
#[test]
fn follow_nonexistent_relative_import_simple() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", ""),
("src/package/bar.py", "from .foo import X"),
])?;
assert_public_ty(&db, "src/package/bar.py", "X", "Unknown");
Ok(())
}
#[test]
fn follow_relative_import_dotted() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", ""),
("src/package/foo/bar/baz.py", "X = 42"),
("src/package/bar.py", "from .foo.bar.baz import X"),
])?;
assert_public_ty(&db, "src/package/bar.py", "X", "Literal[42]");
Ok(())
}
#[test]
fn follow_relative_import_bare_to_package() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", "X = 42"),
("src/package/bar.py", "from . import X"),
])?;
assert_public_ty(&db, "src/package/bar.py", "X", "Literal[42]");
Ok(())
}
#[test]
fn follow_nonexistent_relative_import_bare_to_package() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([("src/package/bar.py", "from . import X")])?;
assert_public_ty(&db, "src/package/bar.py", "X", "Unknown");
Ok(())
}
#[ignore = "TODO: Submodule imports possibly not supported right now?"]
#[test]
fn follow_relative_import_bare_to_module() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", ""),
("src/package/foo.py", "X = 42"),
("src/package/bar.py", "from . import foo; y = foo.X"),
])?;
assert_public_ty(&db, "src/package/bar.py", "y", "Literal[42]");
Ok(())
}
#[ignore = "TODO: Submodule imports possibly not supported right now?"]
#[test]
fn follow_nonexistent_import_bare_to_module() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", ""),
("src/package/bar.py", "from . import foo"),
])?;
assert_public_ty(&db, "src/package/bar.py", "foo", "Unknown");
Ok(())
}
#[test]
fn follow_relative_import_from_dunder_init() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", "from .foo import X"),
("src/package/foo.py", "X = 42"),
])?;
assert_public_ty(&db, "src/package/__init__.py", "X", "Literal[42]");
Ok(())
}
#[test]
fn follow_nonexistent_relative_import_from_dunder_init() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([("src/package/__init__.py", "from .foo import X")])?;
assert_public_ty(&db, "src/package/__init__.py", "X", "Unknown");
Ok(())
}
#[test]
fn follow_very_relative_import() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", ""),
("src/package/foo.py", "X = 42"),
(
"src/package/subpackage/subsubpackage/bar.py",
"from ...foo import X",
),
])?;
assert_public_ty(
&db,
"src/package/subpackage/subsubpackage/bar.py",
"X",
"Literal[42]",
);
Ok(())
}
#[test]
fn imported_unbound_symbol_is_unknown() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_files([
("src/package/__init__.py", ""),
("src/package/foo.py", "x"),
("src/package/bar.py", "from package.foo import x"),
])?;
// the type as seen from external modules (`Unknown`)
// is different from the type inside the module itself (`Unbound`):
assert_public_ty(&db, "src/package/foo.py", "x", "Unbound");
assert_public_ty(&db, "src/package/bar.py", "x", "Unknown");
Ok(())
}
#[test]
fn resolve_base_class_by_name() -> anyhow::Result<()> {
let mut db = setup_db();
@@ -2163,6 +2444,38 @@ mod tests {
Ok(())
}
#[test]
fn conditionally_global_or_builtin() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_dedented(
"/src/a.py",
"
if flag:
copyright = 1
def f():
y = copyright
",
)?;
let file = system_path_to_file(&db, "src/a.py").expect("Expected file to exist.");
let index = semantic_index(&db, file);
let function_scope = index
.child_scopes(FileScopeId::global())
.next()
.unwrap()
.0
.to_scope_id(&db, file);
let y_ty = symbol_ty_by_name(&db, function_scope, "y");
assert_eq!(
y_ty.display(&db).to_string(),
"Literal[1] | Literal[copyright]"
);
Ok(())
}
/// Class name lookups do fall back to globals, but the public type never does.
#[test]
fn unbound_class_local() -> anyhow::Result<()> {
@@ -2291,6 +2604,26 @@ mod tests {
Ok(())
}
#[test]
fn narrow_not_none() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_dedented(
"/src/a.py",
"
x = None if flag else 1
y = 0
if x is not None:
y = x
",
)?;
assert_public_ty(&db, "/src/a.py", "x", "Literal[1] | None");
assert_public_ty(&db, "/src/a.py", "y", "Literal[0, 1]");
Ok(())
}
#[test]
fn while_loop() -> anyhow::Result<()> {
let mut db = setup_db();
@@ -2388,10 +2721,11 @@ mod tests {
fn first_public_def<'db>(db: &'db TestDb, file: File, name: &str) -> Definition<'db> {
let scope = global_scope(db, file);
*use_def_map(db, scope)
use_def_map(db, scope)
.public_definitions(symbol_table(db, scope).symbol_id_by_name(name).unwrap())
.first()
.next()
.unwrap()
.definition
}
#[test]

View File

@@ -0,0 +1,115 @@
use crate::semantic_index::ast_ids::HasScopedAstId;
use crate::semantic_index::definition::Definition;
use crate::semantic_index::expression::Expression;
use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId, SymbolTable};
use crate::semantic_index::symbol_table;
use crate::types::{infer_expression_types, IntersectionBuilder, Type, TypeInference};
use crate::Db;
use ruff_python_ast as ast;
use rustc_hash::FxHashMap;
use std::sync::Arc;
/// Return the type constraint that `test` (if true) would place on `definition`, if any.
///
/// For example, if we have this code:
///
/// ```python
/// y = 1 if flag else None
/// x = 1 if flag else None
/// if x is not None:
/// ...
/// ```
///
/// The `test` expression `x is not None` places the constraint "not None" on the definition of
/// `x`, so in that case we'd return `Some(Type::Intersection(negative=[Type::None]))`.
///
/// But if we called this with the same `test` expression, but the `definition` of `y`, no
/// constraint is applied to that definition, so we'd just return `None`.
pub(crate) fn narrowing_constraint<'db>(
db: &'db dyn Db,
test: Expression<'db>,
definition: Definition<'db>,
) -> Option<Type<'db>> {
all_narrowing_constraints(db, test)
.get(&definition.symbol(db))
.copied()
}
#[salsa::tracked(return_ref)]
fn all_narrowing_constraints<'db>(
db: &'db dyn Db,
test: Expression<'db>,
) -> NarrowingConstraints<'db> {
NarrowingConstraintsBuilder::new(db, test).finish()
}
type NarrowingConstraints<'db> = FxHashMap<ScopedSymbolId, Type<'db>>;
struct NarrowingConstraintsBuilder<'db> {
db: &'db dyn Db,
expression: Expression<'db>,
constraints: NarrowingConstraints<'db>,
}
impl<'db> NarrowingConstraintsBuilder<'db> {
fn new(db: &'db dyn Db, expression: Expression<'db>) -> Self {
Self {
db,
expression,
constraints: NarrowingConstraints::default(),
}
}
fn finish(mut self) -> NarrowingConstraints<'db> {
if let ast::Expr::Compare(expr_compare) = self.expression.node_ref(self.db).node() {
self.add_expr_compare(expr_compare);
}
// TODO other test expression kinds
self.constraints.shrink_to_fit();
self.constraints
}
fn symbols(&self) -> Arc<SymbolTable> {
symbol_table(self.db, self.scope())
}
fn scope(&self) -> ScopeId<'db> {
self.expression.scope(self.db)
}
fn inference(&self) -> &'db TypeInference<'db> {
infer_expression_types(self.db, self.expression)
}
fn add_expr_compare(&mut self, expr_compare: &ast::ExprCompare) {
let ast::ExprCompare {
range: _,
left,
ops,
comparators,
} = expr_compare;
if let ast::Expr::Name(ast::ExprName {
range: _,
id,
ctx: _,
}) = left.as_ref()
{
// SAFETY: we should always have a symbol for every Name node.
let symbol = self.symbols().symbol_id_by_name(id).unwrap();
let scope = self.scope();
let inference = self.inference();
for (op, comparator) in std::iter::zip(&**ops, &**comparators) {
let comp_ty = inference.expression_ty(comparator.scoped_ast_id(self.db, scope));
if matches!(op, ast::CmpOp::IsNot) {
let ty = IntersectionBuilder::new(self.db)
.add_negative(comp_ty)
.build();
self.constraints.insert(symbol, ty);
};
// TODO other comparison types
}
}
}
}

View File

@@ -124,12 +124,16 @@ fn format_diagnostic(context: &SemanticLintContext, message: &str, start: TextSi
}
fn lint_unresolved_imports(context: &SemanticLintContext, import: AnyImportRef) {
// TODO: this treats any symbol with `Type::Unknown` as an unresolved import,
// which isn't really correct: if it exists but has `Type::Unknown` in the
// module we're importing it from, we shouldn't really emit a diagnostic here,
// but currently do.
match import {
AnyImportRef::Import(import) => {
for alias in &import.names {
let ty = alias.ty(&context.semantic);
if ty.is_unbound() {
if ty.is_unknown() {
context.push_diagnostic(format_diagnostic(
context,
&format!("Unresolved import '{}'", &alias.name),
@@ -142,7 +146,7 @@ fn lint_unresolved_imports(context: &SemanticLintContext, import: AnyImportRef)
for alias in &import.names {
let ty = alias.ty(&context.semantic);
if ty.is_unbound() {
if ty.is_unknown() {
context.push_diagnostic(format_diagnostic(
context,
&format!("Unresolved import '{}'", &alias.name),

View File

@@ -1,9 +1,14 @@
use red_knot_python_semantic::{ProgramSettings, PythonVersion, SearchPathSettings};
use red_knot_python_semantic::{
HasTy, ProgramSettings, PythonVersion, SearchPathSettings, SemanticModel,
};
use red_knot_workspace::db::RootDatabase;
use red_knot_workspace::lint::lint_semantic;
use red_knot_workspace::workspace::WorkspaceMetadata;
use ruff_db::files::system_path_to_file;
use ruff_db::system::{OsSystem, SystemPathBuf};
use ruff_db::files::{system_path_to_file, File};
use ruff_db::parsed::parsed_module;
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
use ruff_python_ast::visitor::source_order;
use ruff_python_ast::visitor::source_order::SourceOrderVisitor;
use ruff_python_ast::{Alias, Expr, Parameter, ParameterWithDefault, Stmt};
use std::fs;
use std::path::PathBuf;
@@ -28,17 +33,100 @@ fn setup_db(workspace_root: SystemPathBuf) -> anyhow::Result<RootDatabase> {
#[allow(clippy::print_stdout)]
fn corpus_no_panic() -> anyhow::Result<()> {
let corpus = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("resources/test/corpus");
let system_corpus =
SystemPathBuf::from_path_buf(corpus.clone()).expect("corpus path to be UTF8");
let db = setup_db(system_corpus.clone())?;
let system_corpus = SystemPath::from_std_path(&corpus).expect("corpus path to be UTF8");
let db = setup_db(system_corpus.to_path_buf())?;
for path in fs::read_dir(&corpus).expect("corpus to be a directory") {
let path = path.expect("path to not be an error").path();
println!("checking {path:?}");
let path = SystemPathBuf::from_path_buf(path.clone()).expect("path to be UTF-8");
// this test is only asserting that we can run the lint without a panic
// this test is only asserting that we can pull every expression type without a panic
// (and some non-expressions that clearly define a single type)
let file = system_path_to_file(&db, path).expect("file to exist");
lint_semantic(&db, file);
pull_types(&db, file);
}
Ok(())
}
fn pull_types(db: &RootDatabase, file: File) {
let mut visitor = PullTypesVisitor::new(db, file);
let ast = parsed_module(db, file);
visitor.visit_body(ast.suite());
}
struct PullTypesVisitor<'db> {
model: SemanticModel<'db>,
}
impl<'db> PullTypesVisitor<'db> {
fn new(db: &'db RootDatabase, file: File) -> Self {
Self {
model: SemanticModel::new(db, file),
}
}
}
impl SourceOrderVisitor<'_> for PullTypesVisitor<'_> {
fn visit_stmt(&mut self, stmt: &Stmt) {
match stmt {
Stmt::FunctionDef(function) => {
let _ty = function.ty(&self.model);
}
Stmt::ClassDef(class) => {
let _ty = class.ty(&self.model);
}
Stmt::AnnAssign(_)
| Stmt::Return(_)
| Stmt::Delete(_)
| Stmt::Assign(_)
| Stmt::AugAssign(_)
| Stmt::TypeAlias(_)
| Stmt::For(_)
| Stmt::While(_)
| Stmt::If(_)
| Stmt::With(_)
| Stmt::Match(_)
| Stmt::Raise(_)
| Stmt::Try(_)
| Stmt::Assert(_)
| Stmt::Import(_)
| Stmt::ImportFrom(_)
| Stmt::Global(_)
| Stmt::Nonlocal(_)
| Stmt::Expr(_)
| Stmt::Pass(_)
| Stmt::Break(_)
| Stmt::Continue(_)
| Stmt::IpyEscapeCommand(_) => {}
}
source_order::walk_stmt(self, stmt);
}
fn visit_expr(&mut self, expr: &Expr) {
let _ty = expr.ty(&self.model);
source_order::walk_expr(self, expr);
}
fn visit_parameter(&mut self, parameter: &Parameter) {
let _ty = parameter.ty(&self.model);
source_order::walk_parameter(self, parameter);
}
fn visit_parameter_with_default(&mut self, parameter_with_default: &ParameterWithDefault) {
let _ty = parameter_with_default.ty(&self.model);
source_order::walk_parameter_with_default(self, parameter_with_default);
}
fn visit_alias(&mut self, alias: &Alias) {
let _ty = alias.ty(&self.model);
source_order::walk_alias(self, alias);
}
}

View File

@@ -1,6 +1,6 @@
[package]
name = "ruff"
version = "0.6.0"
version = "0.6.1"
publish = true
authors = { workspace = true }
edition = { workspace = true }

View File

@@ -1434,7 +1434,7 @@ def unused(x):
insta::assert_snapshot!(test_code, @r###"
def unused(x): # noqa: ANN001, ANN201, ARG001, D103
def unused(x): # noqa: ANN001, ANN201, D103
pass
"###);

View File

@@ -89,7 +89,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
let Case { db, parser, .. } = case;
let result = db.check_file(*parser).unwrap();
assert_eq!(result.len(), 402);
assert_eq!(result.len(), 34);
},
BatchSize::SmallInput,
);
@@ -104,7 +104,7 @@ fn benchmark_cold(criterion: &mut Criterion) {
let Case { db, parser, .. } = case;
let result = db.check_file(*parser).unwrap();
assert_eq!(result.len(), 402);
assert_eq!(result.len(), 34);
},
BatchSize::SmallInput,
);

View File

@@ -1,6 +1,6 @@
[package]
name = "ruff_linter"
version = "0.6.0"
version = "0.6.1"
publish = false
authors = { workspace = true }
edition = { workspace = true }

View File

@@ -0,0 +1,134 @@
from fastapi import FastAPI
app = FastAPI()
# Errors
@app.get("/things/{thing_id}")
async def read_thing(query: str):
return {"query": query}
@app.get("/books/isbn-{isbn}")
async def read_thing():
...
@app.get("/things/{thing_id:path}")
async def read_thing(query: str):
return {"query": query}
@app.get("/things/{thing_id : path}")
async def read_thing(query: str):
return {"query": query}
@app.get("/books/{author}/{title}")
async def read_thing(author: str):
return {"author": author}
@app.get("/books/{author_name}/{title}")
async def read_thing():
...
@app.get("/books/{author}/{title}")
async def read_thing(author: str, title: str, /):
return {"author": author, "title": title}
@app.get("/books/{author}/{title}/{page}")
async def read_thing(
author: str,
query: str,
): ...
@app.get("/books/{author}/{title}")
async def read_thing():
...
@app.get("/books/{author}/{title}")
async def read_thing(*, author: str):
...
@app.get("/books/{author}/{title}")
async def read_thing(hello, /, *, author: str):
...
@app.get("/things/{thing_id}")
async def read_thing(
query: str,
):
return {"query": query}
@app.get("/things/{thing_id}")
async def read_thing(
query: str = "default",
):
return {"query": query}
@app.get("/things/{thing_id}")
async def read_thing(
*, query: str = "default",
):
return {"query": query}
# OK
@app.get("/things/{thing_id}")
async def read_thing(thing_id: int, query: str):
return {"thing_id": thing_id, "query": query}
@app.get("/books/isbn-{isbn}")
async def read_thing(isbn: str):
return {"isbn": isbn}
@app.get("/things/{thing_id:path}")
async def read_thing(thing_id: str, query: str):
return {"thing_id": thing_id, "query": query}
@app.get("/things/{thing_id : path}")
async def read_thing(thing_id: str, query: str):
return {"thing_id": thing_id, "query": query}
@app.get("/books/{author}/{title}")
async def read_thing(author: str, title: str):
return {"author": author, "title": title}
@app.get("/books/{author}/{title}")
async def read_thing(*, author: str, title: str):
return {"author": author, "title": title}
@app.get("/books/{author}/{title:path}")
async def read_thing(*, author: str, title: str):
return {"author": author, "title": title}
# Ignored
@app.get("/things/{thing-id}")
async def read_thing(query: str):
return {"query": query}
@app.get("/things/{thing_id!r}")
async def read_thing(query: str):
return {"query": query}
@app.get("/things/{thing_id=}")
async def read_thing(query: str):
return {"query": query}

View File

@@ -17,6 +17,11 @@ def test():
1 in (1, 2)
def test2():
1 in (1, 2)
return
data = [x for x in [1, 2, 3] if x in (1, 2)]

View File

@@ -1,2 +1,10 @@
import mod.CaMel as CM
from mod import CamelCase as CC
# OK depending on configured import convention
import xml.etree.ElementTree as ET
from xml.etree import ElementTree as ET
# Always an error (relative import)
from ..xml.eltree import ElementTree as ET

View File

@@ -42,3 +42,9 @@ class Foo:
@classmethod
def graze(cls, x, y, z):
pass
class Foo:
@staticmethod
def __new__(cls, x, y, z): # OK, see https://docs.python.org/3/reference/datamodel.html#basic-customization
pass

View File

@@ -3,7 +3,7 @@ class Fruit:
def list_fruits(cls) -> None:
cls = "apple" # PLW0642
cls: Fruit = "apple" # PLW0642
cls += "orange" # PLW0642
cls += "orange" # OK, augmented assignments are ignored
*cls = "banana" # PLW0642
cls, blah = "apple", "orange" # PLW0642
blah, (cls, blah2) = "apple", ("orange", "banana") # PLW0642
@@ -16,7 +16,7 @@ class Fruit:
def print_color(self) -> None:
self = "red" # PLW0642
self: Self = "red" # PLW0642
self += "blue" # PLW0642
self += "blue" # OK, augmented assignments are ignored
*self = "blue" # PLW0642
self, blah = "red", "blue" # PLW0642
blah, (self, blah2) = "apple", ("orange", "banana") # PLW0642

View File

@@ -59,3 +59,12 @@ def negative_cases():
# See https://docs.python.org/3/howto/logging-cookbook.html#formatting-styles
import logging
logging.info("yet {another} non-f-string")
# See https://fastapi.tiangolo.com/tutorial/path-params/
from fastapi import FastAPI
app = FastAPI()
item_id = 42
@app.get("/items/{item_id}")
async def read_item(item_id):
return {"item_id": item_id}

View File

@@ -78,3 +78,13 @@ async def test():
async def test() -> str:
vals = [str(val) for val in await async_func(1)]
return ",".join(vals)
from fastapi import FastAPI
app = FastAPI()
@app.post("/count")
async def fastapi_route(): # Ok: FastApi routes can be async without actually using await
return 1

View File

@@ -94,6 +94,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::FastApiNonAnnotatedDependency) {
fastapi::rules::fastapi_non_annotated_dependency(checker, function_def);
}
if checker.enabled(Rule::FastApiUnusedPathParameter) {
fastapi::rules::fastapi_unused_path_parameter(checker, function_def);
}
if checker.enabled(Rule::AmbiguousFunctionName) {
if let Some(diagnostic) = pycodestyle::rules::ambiguous_function_name(name) {
checker.diagnostics.push(diagnostic);
@@ -704,11 +707,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
if checker.enabled(Rule::CamelcaseImportedAsAcronym) {
if let Some(diagnostic) = pep8_naming::rules::camelcase_imported_as_acronym(
name,
asname,
alias,
stmt,
&checker.settings.pep8_naming.ignore_names,
name, asname, alias, stmt, checker,
) {
checker.diagnostics.push(diagnostic);
}
@@ -1023,7 +1022,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
asname,
alias,
stmt,
&checker.settings.pep8_naming.ignore_names,
checker,
) {
checker.diagnostics.push(diagnostic);
}
@@ -1113,9 +1112,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
}
}
Stmt::AugAssign(aug_assign @ ast::StmtAugAssign { target, .. }) => {
if checker.enabled(Rule::SelfOrClsAssignment) {
pylint::rules::self_or_cls_assignment(checker, target);
}
if checker.enabled(Rule::GlobalStatement) {
if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() {
pylint::rules::global_statement(checker, id);

View File

@@ -920,6 +920,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
// fastapi
(FastApi, "001") => (RuleGroup::Preview, rules::fastapi::rules::FastApiRedundantResponseModel),
(FastApi, "002") => (RuleGroup::Preview, rules::fastapi::rules::FastApiNonAnnotatedDependency),
(FastApi, "003") => (RuleGroup::Preview, rules::fastapi::rules::FastApiUnusedPathParameter),
// pydoclint
(Pydoclint, "201") => (RuleGroup::Preview, rules::pydoclint::rules::DocstringMissingReturns),

View File

@@ -4,7 +4,7 @@ use anyhow::{Context, Result};
use ruff_diagnostics::Edit;
use ruff_python_ast::parenthesize::parenthesized_range;
use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Expr, ExprList, Stmt};
use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Expr, ExprList, Parameters, Stmt};
use ruff_python_ast::{AnyNodeRef, ArgOrKeyword};
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
@@ -282,6 +282,59 @@ pub(crate) fn add_argument(
}
}
/// Generic function to add a (regular) parameter to a function definition.
pub(crate) fn add_parameter(parameter: &str, parameters: &Parameters, source: &str) -> Edit {
if let Some(last) = parameters
.args
.iter()
.filter(|arg| arg.default.is_none())
.last()
{
// Case 1: at least one regular parameter, so append after the last one.
Edit::insertion(format!(", {parameter}"), last.end())
} else if parameters.args.first().is_some() {
// Case 2: no regular parameters, but at least one keyword parameter, so add before the
// first.
let pos = parameters.start();
let mut tokenizer = SimpleTokenizer::starts_at(pos, source);
let name = tokenizer
.find(|token| token.kind == SimpleTokenKind::Name)
.expect("Unable to find name token");
Edit::insertion(format!("{parameter}, "), name.start())
} else if let Some(last) = parameters.posonlyargs.last() {
// Case 2: no regular parameter, but a positional-only parameter exists, so add after that.
// We take care to add it *after* the `/` separator.
let pos = last.end();
let mut tokenizer = SimpleTokenizer::starts_at(pos, source);
let slash = tokenizer
.find(|token| token.kind == SimpleTokenKind::Slash)
.expect("Unable to find `/` token");
// Try to find a comma after the slash.
let comma = tokenizer.find(|token| token.kind == SimpleTokenKind::Comma);
if let Some(comma) = comma {
Edit::insertion(format!(" {parameter},"), comma.start() + TextSize::from(1))
} else {
Edit::insertion(format!(", {parameter}"), slash.start())
}
} else if parameters.kwonlyargs.first().is_some() {
// Case 3: no regular parameter, but a keyword-only parameter exist, so add parameter before that.
// We need to backtrack to before the `*` separator.
// We know there is no non-keyword-only params, so we can safely assume that the `*` separator is the first
let pos = parameters.start();
let mut tokenizer = SimpleTokenizer::starts_at(pos, source);
let star = tokenizer
.find(|token| token.kind == SimpleTokenKind::Star)
.expect("Unable to find `*` token");
Edit::insertion(format!("{parameter}, "), star.start())
} else {
// Case 4: no parameters at all, so add parameter after the opening parenthesis.
Edit::insertion(
parameter.to_string(),
parameters.start() + TextSize::from(1),
)
}
}
/// Safely adjust the indentation of the indented block at [`TextRange`].
///
/// The [`TextRange`] is assumed to represent an entire indented block, including the leading

View File

@@ -15,6 +15,7 @@ mod tests {
#[test_case(Rule::FastApiRedundantResponseModel, Path::new("FAST001.py"))]
#[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002.py"))]
#[test_case(Rule::FastApiUnusedPathParameter, Path::new("FAST003.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
let diagnostics = test_path(

View File

@@ -0,0 +1,232 @@
use std::iter::Peekable;
use std::ops::Range;
use std::str::CharIndices;
use ruff_diagnostics::Fix;
use ruff_diagnostics::{Diagnostic, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_semantic::Modules;
use ruff_python_stdlib::identifiers::is_identifier;
use ruff_text_size::{Ranged, TextSize};
use crate::checkers::ast::Checker;
use crate::fix::edits::add_parameter;
use crate::rules::fastapi::rules::is_fastapi_route_decorator;
/// ## What it does
/// Identifies FastAPI routes that declare path parameters in the route path
/// that are not included in the function signature.
///
/// ## Why is this bad?
/// Path parameters are used to extract values from the URL path.
///
/// If a path parameter is declared in the route path but not in the function
/// signature, it will not be accessible in the function body, which is likely
/// a mistake.
///
/// If a path parameter is declared in the route path, but as a positional-only
/// argument in the function signature, it will also not be accessible in the
/// function body, as FastAPI will not inject the parameter.
///
/// ## Known problems
/// If the path parameter is _not_ a valid Python identifier (e.g., `user-id`, as
/// opposed to `user_id`), FastAPI will normalize it. However, this rule simply
/// ignores such path parameters, as FastAPI's normalization behavior is undocumented.
///
/// ## Example
///
/// ```python
/// from fastapi import FastAPI
///
/// app = FastAPI()
///
///
/// @app.get("/things/{thing_id}")
/// async def read_thing(query: str): ...
/// ```
///
/// Use instead:
///
/// ```python
/// from fastapi import FastAPI
///
/// app = FastAPI()
///
///
/// @app.get("/things/{thing_id}")
/// async def read_thing(thing_id: int, query: str): ...
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as unsafe, as modifying a function signature can
/// change the behavior of the code.
#[violation]
pub struct FastApiUnusedPathParameter {
arg_name: String,
function_name: String,
is_positional: bool,
}
impl Violation for FastApiUnusedPathParameter {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
let Self {
arg_name,
function_name,
is_positional,
} = self;
#[allow(clippy::if_not_else)]
if !is_positional {
format!("Parameter `{arg_name}` appears in route path, but not in `{function_name}` signature")
} else {
format!(
"Parameter `{arg_name}` appears in route path, but only as a positional-only argument in `{function_name}` signature"
)
}
}
fn fix_title(&self) -> Option<String> {
let Self {
arg_name,
is_positional,
..
} = self;
if *is_positional {
None
} else {
Some(format!("Add `{arg_name}` to function signature"))
}
}
}
/// FAST003
pub(crate) fn fastapi_unused_path_parameter(
checker: &mut Checker,
function_def: &ast::StmtFunctionDef,
) {
if !checker.semantic().seen_module(Modules::FASTAPI) {
return;
}
// Get the route path from the decorator.
let route_decorator = function_def
.decorator_list
.iter()
.find_map(|decorator| is_fastapi_route_decorator(decorator, checker.semantic()));
let Some(route_decorator) = route_decorator else {
return;
};
let Some(path_arg) = route_decorator.arguments.args.first() else {
return;
};
let diagnostic_range = path_arg.range();
// We can't really handle anything other than string literals.
let path = match path_arg.as_string_literal_expr() {
Some(path_arg) => &path_arg.value,
None => return,
};
// Extract the path parameters from the route path.
let path_params = PathParamIterator::new(path.to_str());
// Extract the arguments from the function signature
let named_args: Vec<_> = function_def
.parameters
.args
.iter()
.chain(function_def.parameters.kwonlyargs.iter())
.map(|arg| arg.parameter.name.as_str())
.collect();
// Check if any of the path parameters are not in the function signature.
let mut diagnostics = vec![];
for (path_param, range) in path_params {
// Ignore invalid identifiers (e.g., `user-id`, as opposed to `user_id`)
if !is_identifier(path_param) {
continue;
}
// If the path parameter is already in the function signature, we don't need to do anything.
if named_args.contains(&path_param) {
continue;
}
// Determine whether the path parameter is used as a positional-only argument. In this case,
// the path parameter injection won't work, but we also can't fix it (yet), since we'd need
// to make the parameter non-positional-only.
let is_positional = function_def
.parameters
.posonlyargs
.iter()
.any(|arg| arg.parameter.name.as_str() == path_param);
let mut diagnostic = Diagnostic::new(
FastApiUnusedPathParameter {
arg_name: path_param.to_string(),
function_name: function_def.name.to_string(),
is_positional,
},
#[allow(clippy::cast_possible_truncation)]
diagnostic_range
.add_start(TextSize::from(range.start as u32 + 1))
.sub_end(TextSize::from((path.len() - range.end + 1) as u32)),
);
if !is_positional {
diagnostic.set_fix(Fix::unsafe_edit(add_parameter(
path_param,
&function_def.parameters,
checker.locator().contents(),
)));
}
diagnostics.push(diagnostic);
}
checker.diagnostics.extend(diagnostics);
}
/// An iterator to extract parameters from FastAPI route paths.
///
/// The iterator yields tuples of the parameter name and the range of the parameter in the input,
/// inclusive of curly braces.
#[derive(Debug)]
struct PathParamIterator<'a> {
input: &'a str,
chars: Peekable<CharIndices<'a>>,
}
impl<'a> PathParamIterator<'a> {
fn new(input: &'a str) -> Self {
PathParamIterator {
input,
chars: input.char_indices().peekable(),
}
}
}
impl<'a> Iterator for PathParamIterator<'a> {
type Item = (&'a str, Range<usize>);
fn next(&mut self) -> Option<Self::Item> {
while let Some((start, c)) = self.chars.next() {
if c == '{' {
if let Some((end, _)) = self.chars.by_ref().find(|&(_, ch)| ch == '}') {
let param_content = &self.input[start + 1..end];
// We ignore text after a colon, since those are path convertors
// See also: https://fastapi.tiangolo.com/tutorial/path-params/?h=path#path-convertor
let param_name_end = param_content.find(':').unwrap_or(param_content.len());
let param_name = &param_content[..param_name_end].trim();
#[allow(clippy::range_plus_one)]
return Some((param_name, start..end + 1));
}
}
}
None
}
}

View File

@@ -1,15 +1,20 @@
pub(crate) use fastapi_non_annotated_dependency::*;
pub(crate) use fastapi_redundant_response_model::*;
pub(crate) use fastapi_unused_path_parameter::*;
mod fastapi_non_annotated_dependency;
mod fastapi_redundant_response_model;
mod fastapi_unused_path_parameter;
use ruff_python_ast::{Decorator, ExprCall, StmtFunctionDef};
use ruff_python_ast as ast;
use ruff_python_semantic::analyze::typing::resolve_assignment;
use ruff_python_semantic::SemanticModel;
/// Returns `true` if the function is a FastAPI route.
pub(crate) fn is_fastapi_route(function_def: &StmtFunctionDef, semantic: &SemanticModel) -> bool {
pub(crate) fn is_fastapi_route(
function_def: &ast::StmtFunctionDef,
semantic: &SemanticModel,
) -> bool {
return function_def
.decorator_list
.iter()
@@ -18,27 +23,29 @@ pub(crate) fn is_fastapi_route(function_def: &StmtFunctionDef, semantic: &Semant
/// Returns `true` if the decorator is indicative of a FastAPI route.
pub(crate) fn is_fastapi_route_decorator<'a>(
decorator: &'a Decorator,
decorator: &'a ast::Decorator,
semantic: &'a SemanticModel,
) -> Option<&'a ExprCall> {
) -> Option<&'a ast::ExprCall> {
let call = decorator.expression.as_call_expr()?;
let decorator_method = call.func.as_attribute_expr()?;
let method_name = &decorator_method.attr;
is_fastapi_route_call(call, semantic).then_some(call)
}
pub(crate) fn is_fastapi_route_call(call_expr: &ast::ExprCall, semantic: &SemanticModel) -> bool {
let ast::Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = &*call_expr.func else {
return false;
};
if !matches!(
method_name.as_str(),
attr.as_str(),
"get" | "post" | "put" | "delete" | "patch" | "options" | "head" | "trace"
) {
return None;
return false;
}
let qualified_name = resolve_assignment(&decorator_method.value, semantic)?;
if matches!(
qualified_name.segments(),
["fastapi", "FastAPI" | "APIRouter"]
) {
Some(call)
} else {
None
}
resolve_assignment(value, semantic).is_some_and(|qualified_name| {
matches!(
qualified_name.segments(),
["fastapi", "FastAPI" | "APIRouter"]
)
})
}

View File

@@ -0,0 +1,323 @@
---
source: crates/ruff_linter/src/rules/fastapi/mod.rs
---
FAST003.py:7:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing` signature
|
6 | # Errors
7 | @app.get("/things/{thing_id}")
| ^^^^^^^^^^ FAST003
8 | async def read_thing(query: str):
9 | return {"query": query}
|
= help: Add `thing_id` to function signature
Unsafe fix
5 5 |
6 6 | # Errors
7 7 | @app.get("/things/{thing_id}")
8 |-async def read_thing(query: str):
8 |+async def read_thing(query: str, thing_id):
9 9 | return {"query": query}
10 10 |
11 11 |
FAST003.py:12:23: FAST003 [*] Parameter `isbn` appears in route path, but not in `read_thing` signature
|
12 | @app.get("/books/isbn-{isbn}")
| ^^^^^^ FAST003
13 | async def read_thing():
14 | ...
|
= help: Add `isbn` to function signature
Unsafe fix
10 10 |
11 11 |
12 12 | @app.get("/books/isbn-{isbn}")
13 |-async def read_thing():
13 |+async def read_thing(isbn):
14 14 | ...
15 15 |
16 16 |
FAST003.py:17:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing` signature
|
17 | @app.get("/things/{thing_id:path}")
| ^^^^^^^^^^^^^^^ FAST003
18 | async def read_thing(query: str):
19 | return {"query": query}
|
= help: Add `thing_id` to function signature
Unsafe fix
15 15 |
16 16 |
17 17 | @app.get("/things/{thing_id:path}")
18 |-async def read_thing(query: str):
18 |+async def read_thing(query: str, thing_id):
19 19 | return {"query": query}
20 20 |
21 21 |
FAST003.py:22:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing` signature
|
22 | @app.get("/things/{thing_id : path}")
| ^^^^^^^^^^^^^^^^^ FAST003
23 | async def read_thing(query: str):
24 | return {"query": query}
|
= help: Add `thing_id` to function signature
Unsafe fix
20 20 |
21 21 |
22 22 | @app.get("/things/{thing_id : path}")
23 |-async def read_thing(query: str):
23 |+async def read_thing(query: str, thing_id):
24 24 | return {"query": query}
25 25 |
26 26 |
FAST003.py:27:27: FAST003 [*] Parameter `title` appears in route path, but not in `read_thing` signature
|
27 | @app.get("/books/{author}/{title}")
| ^^^^^^^ FAST003
28 | async def read_thing(author: str):
29 | return {"author": author}
|
= help: Add `title` to function signature
Unsafe fix
25 25 |
26 26 |
27 27 | @app.get("/books/{author}/{title}")
28 |-async def read_thing(author: str):
28 |+async def read_thing(author: str, title):
29 29 | return {"author": author}
30 30 |
31 31 |
FAST003.py:32:18: FAST003 [*] Parameter `author_name` appears in route path, but not in `read_thing` signature
|
32 | @app.get("/books/{author_name}/{title}")
| ^^^^^^^^^^^^^ FAST003
33 | async def read_thing():
34 | ...
|
= help: Add `author_name` to function signature
Unsafe fix
30 30 |
31 31 |
32 32 | @app.get("/books/{author_name}/{title}")
33 |-async def read_thing():
33 |+async def read_thing(author_name):
34 34 | ...
35 35 |
36 36 |
FAST003.py:32:32: FAST003 [*] Parameter `title` appears in route path, but not in `read_thing` signature
|
32 | @app.get("/books/{author_name}/{title}")
| ^^^^^^^ FAST003
33 | async def read_thing():
34 | ...
|
= help: Add `title` to function signature
Unsafe fix
30 30 |
31 31 |
32 32 | @app.get("/books/{author_name}/{title}")
33 |-async def read_thing():
33 |+async def read_thing(title):
34 34 | ...
35 35 |
36 36 |
FAST003.py:37:18: FAST003 Parameter `author` appears in route path, but only as a positional-only argument in `read_thing` signature
|
37 | @app.get("/books/{author}/{title}")
| ^^^^^^^^ FAST003
38 | async def read_thing(author: str, title: str, /):
39 | return {"author": author, "title": title}
|
FAST003.py:37:27: FAST003 Parameter `title` appears in route path, but only as a positional-only argument in `read_thing` signature
|
37 | @app.get("/books/{author}/{title}")
| ^^^^^^^ FAST003
38 | async def read_thing(author: str, title: str, /):
39 | return {"author": author, "title": title}
|
FAST003.py:42:27: FAST003 [*] Parameter `title` appears in route path, but not in `read_thing` signature
|
42 | @app.get("/books/{author}/{title}/{page}")
| ^^^^^^^ FAST003
43 | async def read_thing(
44 | author: str,
|
= help: Add `title` to function signature
Unsafe fix
42 42 | @app.get("/books/{author}/{title}/{page}")
43 43 | async def read_thing(
44 44 | author: str,
45 |- query: str,
45 |+ query: str, title,
46 46 | ): ...
47 47 |
48 48 |
FAST003.py:42:35: FAST003 [*] Parameter `page` appears in route path, but not in `read_thing` signature
|
42 | @app.get("/books/{author}/{title}/{page}")
| ^^^^^^ FAST003
43 | async def read_thing(
44 | author: str,
|
= help: Add `page` to function signature
Unsafe fix
42 42 | @app.get("/books/{author}/{title}/{page}")
43 43 | async def read_thing(
44 44 | author: str,
45 |- query: str,
45 |+ query: str, page,
46 46 | ): ...
47 47 |
48 48 |
FAST003.py:49:18: FAST003 [*] Parameter `author` appears in route path, but not in `read_thing` signature
|
49 | @app.get("/books/{author}/{title}")
| ^^^^^^^^ FAST003
50 | async def read_thing():
51 | ...
|
= help: Add `author` to function signature
Unsafe fix
47 47 |
48 48 |
49 49 | @app.get("/books/{author}/{title}")
50 |-async def read_thing():
50 |+async def read_thing(author):
51 51 | ...
52 52 |
53 53 |
FAST003.py:49:27: FAST003 [*] Parameter `title` appears in route path, but not in `read_thing` signature
|
49 | @app.get("/books/{author}/{title}")
| ^^^^^^^ FAST003
50 | async def read_thing():
51 | ...
|
= help: Add `title` to function signature
Unsafe fix
47 47 |
48 48 |
49 49 | @app.get("/books/{author}/{title}")
50 |-async def read_thing():
50 |+async def read_thing(title):
51 51 | ...
52 52 |
53 53 |
FAST003.py:54:27: FAST003 [*] Parameter `title` appears in route path, but not in `read_thing` signature
|
54 | @app.get("/books/{author}/{title}")
| ^^^^^^^ FAST003
55 | async def read_thing(*, author: str):
56 | ...
|
= help: Add `title` to function signature
Unsafe fix
52 52 |
53 53 |
54 54 | @app.get("/books/{author}/{title}")
55 |-async def read_thing(*, author: str):
55 |+async def read_thing(title, *, author: str):
56 56 | ...
57 57 |
58 58 |
FAST003.py:59:27: FAST003 [*] Parameter `title` appears in route path, but not in `read_thing` signature
|
59 | @app.get("/books/{author}/{title}")
| ^^^^^^^ FAST003
60 | async def read_thing(hello, /, *, author: str):
61 | ...
|
= help: Add `title` to function signature
Unsafe fix
57 57 |
58 58 |
59 59 | @app.get("/books/{author}/{title}")
60 |-async def read_thing(hello, /, *, author: str):
60 |+async def read_thing(hello, /, title, *, author: str):
61 61 | ...
62 62 |
63 63 |
FAST003.py:64:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing` signature
|
64 | @app.get("/things/{thing_id}")
| ^^^^^^^^^^ FAST003
65 | async def read_thing(
66 | query: str,
|
= help: Add `thing_id` to function signature
Unsafe fix
63 63 |
64 64 | @app.get("/things/{thing_id}")
65 65 | async def read_thing(
66 |- query: str,
66 |+ query: str, thing_id,
67 67 | ):
68 68 | return {"query": query}
69 69 |
FAST003.py:71:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing` signature
|
71 | @app.get("/things/{thing_id}")
| ^^^^^^^^^^ FAST003
72 | async def read_thing(
73 | query: str = "default",
|
= help: Add `thing_id` to function signature
Unsafe fix
70 70 |
71 71 | @app.get("/things/{thing_id}")
72 72 | async def read_thing(
73 |- query: str = "default",
73 |+ thing_id, query: str = "default",
74 74 | ):
75 75 | return {"query": query}
76 76 |
FAST003.py:78:19: FAST003 [*] Parameter `thing_id` appears in route path, but not in `read_thing` signature
|
78 | @app.get("/things/{thing_id}")
| ^^^^^^^^^^ FAST003
79 | async def read_thing(
80 | *, query: str = "default",
|
= help: Add `thing_id` to function signature
Unsafe fix
77 77 |
78 78 | @app.get("/things/{thing_id}")
79 79 | async def read_thing(
80 |- *, query: str = "default",
80 |+ thing_id, *, query: str = "default",
81 81 | ):
82 82 | return {"query": query}
83 83 |

View File

@@ -32,7 +32,7 @@ use crate::rules::flake8_async::helpers::AsyncModule;
///
///
/// async def main():
/// with asyncio.timeout(2):
/// async with asyncio.timeout(2):
/// await long_running_task()
/// ```
///

View File

@@ -22,14 +22,14 @@ use crate::rules::flake8_async::helpers::MethodName;
/// ## Example
/// ```python
/// async def func():
/// with asyncio.timeout(2):
/// async with asyncio.timeout(2):
/// do_something()
/// ```
///
/// Use instead:
/// ```python
/// async def func():
/// with asyncio.timeout(2):
/// async with asyncio.timeout(2):
/// do_something()
/// await awaitable()
/// ```

View File

@@ -1,6 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Expr;
use ruff_python_ast::{Expr, Stmt};
use ruff_python_semantic::ScopeKind;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@@ -33,24 +34,34 @@ use super::super::helpers::at_last_top_level_expression_in_cell;
/// ## References
/// - [Python documentation: `assert` statement](https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement)
#[violation]
pub struct UselessComparison;
pub struct UselessComparison {
at: ComparisonLocationAt,
}
impl Violation for UselessComparison {
#[derive_message_formats]
fn message(&self) -> String {
format!(
"Pointless comparison. Did you mean to assign a value? \
Otherwise, prepend `assert` or remove it."
)
match self.at {
ComparisonLocationAt::MiddleBody => format!(
"Pointless comparison. Did you mean to assign a value? \
Otherwise, prepend `assert` or remove it."
),
ComparisonLocationAt::EndOfFunction => format!(
"Pointless comparison at end of function scope. Did you mean \
to return the expression result?"
),
}
}
}
/// B015
pub(crate) fn useless_comparison(checker: &mut Checker, expr: &Expr) {
if expr.is_compare_expr() {
let semantic = checker.semantic();
if checker.source_type.is_ipynb()
&& at_last_top_level_expression_in_cell(
checker.semantic(),
semantic,
checker.locator(),
checker.cell_offsets(),
)
@@ -58,8 +69,34 @@ pub(crate) fn useless_comparison(checker: &mut Checker, expr: &Expr) {
return;
}
checker
.diagnostics
.push(Diagnostic::new(UselessComparison, expr.range()));
if let ScopeKind::Function(func_def) = semantic.current_scope().kind {
if func_def
.body
.last()
.and_then(Stmt::as_expr_stmt)
.is_some_and(|last_stmt| &*last_stmt.value == expr)
{
checker.diagnostics.push(Diagnostic::new(
UselessComparison {
at: ComparisonLocationAt::EndOfFunction,
},
expr.range(),
));
return;
}
}
checker.diagnostics.push(Diagnostic::new(
UselessComparison {
at: ComparisonLocationAt::MiddleBody,
},
expr.range(),
));
}
}
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
enum ComparisonLocationAt {
MiddleBody,
EndOfFunction,
}

View File

@@ -1,5 +1,6 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
assertion_line: 74
---
B015.py:3:1: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it.
|
@@ -19,7 +20,7 @@ B015.py:7:1: B015 Pointless comparison. Did you mean to assign a value? Otherwis
| ^^^^^^^^^^^ B015
|
B015.py:17:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it.
B015.py:17:5: B015 Pointless comparison at end of function scope. Did you mean to return the expression result?
|
15 | assert 1 in (1, 2)
16 |
@@ -27,11 +28,17 @@ B015.py:17:5: B015 Pointless comparison. Did you mean to assign a value? Otherwi
| ^^^^^^^^^^^ B015
|
B015.py:24:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it.
B015.py:21:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it.
|
23 | class TestClass:
24 | 1 == 1
20 | def test2():
21 | 1 in (1, 2)
| ^^^^^^^^^^^ B015
22 | return
|
B015.py:29:5: B015 Pointless comparison. Did you mean to assign a value? Otherwise, prepend `assert` or remove it.
|
28 | class TestClass:
29 | 1 == 1
| ^^^^^^ B015
|

View File

@@ -344,6 +344,7 @@ pub(crate) fn unused_arguments(
) {
function_type::FunctionType::Function => {
if checker.enabled(Argumentable::Function.rule_code())
&& !function_type::is_stub(function_def, checker.semantic())
&& !visibility::is_overload(decorator_list, checker.semantic())
{
function(

View File

@@ -32,33 +32,3 @@ ARG.py:13:12: ARG001 Unused function argument: `x`
| ^ ARG001
14 | print("Hello, world!")
|
ARG.py:17:7: ARG001 Unused function argument: `self`
|
17 | def f(self, x):
| ^^^^ ARG001
18 | ...
|
ARG.py:17:13: ARG001 Unused function argument: `x`
|
17 | def f(self, x):
| ^ ARG001
18 | ...
|
ARG.py:21:7: ARG001 Unused function argument: `cls`
|
21 | def f(cls, x):
| ^^^ ARG001
22 | ...
|
ARG.py:21:12: ARG001 Unused function argument: `x`
|
21 | def f(cls, x):
| ^ ARG001
22 | ...
|

View File

@@ -8,11 +8,12 @@ mod tests {
use std::path::{Path, PathBuf};
use anyhow::Result;
use rustc_hash::FxHashMap;
use test_case::test_case;
use crate::registry::Rule;
use crate::rules::pep8_naming;
use crate::rules::pep8_naming::settings::IgnoreNames;
use crate::rules::{flake8_import_conventions, pep8_naming};
use crate::test::test_path;
use crate::{assert_messages, settings};
@@ -87,6 +88,25 @@ mod tests {
Ok(())
}
#[test]
fn camelcase_imported_as_incorrect_convention() -> Result<()> {
let diagnostics = test_path(
Path::new("pep8_naming").join("N817.py").as_path(),
&settings::LinterSettings {
flake8_import_conventions: flake8_import_conventions::settings::Settings {
aliases: FxHashMap::from_iter([(
"xml.etree.ElementTree".to_string(),
"XET".to_string(),
)]),
..Default::default()
},
..settings::LinterSettings::for_rule(Rule::CamelcaseImportedAsAcronym)
},
)?;
assert_messages!(diagnostics);
Ok(())
}
#[test]
fn classmethod_decorators() -> Result<()> {
let diagnostics = test_path(

View File

@@ -1,12 +1,11 @@
use ruff_python_ast::{Alias, Stmt};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Alias, Stmt};
use ruff_python_stdlib::str::{self};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::rules::pep8_naming::helpers;
use crate::rules::pep8_naming::settings::IgnoreNames;
/// ## What it does
/// Checks for `CamelCase` imports that are aliased as acronyms.
@@ -23,6 +22,9 @@ use crate::rules::pep8_naming::settings::IgnoreNames;
/// Note that this rule is distinct from `camelcase-imported-as-constant`
/// to accommodate selective enforcement.
///
/// Also note that import aliases following an import convention according to the
/// [`lint.flake8-import-conventions.aliases`] option are allowed.
///
/// ## Example
/// ```python
/// from example import MyClassName as MCN
@@ -34,6 +36,9 @@ use crate::rules::pep8_naming::settings::IgnoreNames;
/// ```
///
/// [PEP 8]: https://peps.python.org/pep-0008/
///
/// ## Options
/// - `lint.flake8-import-conventions.aliases`
#[violation]
pub struct CamelcaseImportedAsAcronym {
name: String,
@@ -54,17 +59,25 @@ pub(crate) fn camelcase_imported_as_acronym(
asname: &str,
alias: &Alias,
stmt: &Stmt,
ignore_names: &IgnoreNames,
checker: &Checker,
) -> Option<Diagnostic> {
if helpers::is_camelcase(name)
&& !str::is_cased_lowercase(asname)
&& str::is_cased_uppercase(asname)
&& helpers::is_acronym(name, asname)
{
let ignore_names = &checker.settings.pep8_naming.ignore_names;
// Ignore any explicitly-allowed names.
if ignore_names.matches(name) || ignore_names.matches(asname) {
return None;
}
// Ignore names that follow a community-agreed import convention.
if is_ignored_because_of_import_convention(asname, stmt, alias, checker) {
return None;
}
let mut diagnostic = Diagnostic::new(
CamelcaseImportedAsAcronym {
name: name.to_string(),
@@ -77,3 +90,34 @@ pub(crate) fn camelcase_imported_as_acronym(
}
None
}
fn is_ignored_because_of_import_convention(
asname: &str,
stmt: &Stmt,
alias: &Alias,
checker: &Checker,
) -> bool {
let full_name = if let Some(import_from) = stmt.as_import_from_stmt() {
// Never test relative imports for exclusion because we can't resolve the full-module name.
let Some(module) = import_from.module.as_ref() else {
return false;
};
if import_from.level != 0 {
return false;
}
std::borrow::Cow::Owned(format!("{module}.{}", alias.name))
} else {
std::borrow::Cow::Borrowed(&*alias.name)
};
// Ignore names that follow a community-agreed import convention.
checker
.settings
.flake8_import_conventions
.aliases
.get(&*full_name)
.map(String::as_str)
== Some(asname)
}

View File

@@ -15,4 +15,9 @@ N817.py:2:17: N817 CamelCase `CamelCase` imported as acronym `CC`
| ^^^^^^^^^^^^^^^ N817
|
N817.py:10:26: N817 CamelCase `ElementTree` imported as acronym `ET`
|
9 | # Always an error (relative import)
10 | from ..xml.eltree import ElementTree as ET
| ^^^^^^^^^^^^^^^^^ N817
|

View File

@@ -0,0 +1,41 @@
---
source: crates/ruff_linter/src/rules/pep8_naming/mod.rs
---
N817.py:1:8: N817 CamelCase `CaMel` imported as acronym `CM`
|
1 | import mod.CaMel as CM
| ^^^^^^^^^^^^^^^ N817
2 | from mod import CamelCase as CC
|
N817.py:2:17: N817 CamelCase `CamelCase` imported as acronym `CC`
|
1 | import mod.CaMel as CM
2 | from mod import CamelCase as CC
| ^^^^^^^^^^^^^^^ N817
|
N817.py:6:8: N817 CamelCase `ElementTree` imported as acronym `ET`
|
5 | # OK depending on configured import convention
6 | import xml.etree.ElementTree as ET
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ N817
7 | from xml.etree import ElementTree as ET
|
N817.py:7:23: N817 CamelCase `ElementTree` imported as acronym `ET`
|
5 | # OK depending on configured import convention
6 | import xml.etree.ElementTree as ET
7 | from xml.etree import ElementTree as ET
| ^^^^^^^^^^^^^^^^^ N817
8 |
9 | # Always an error (relative import)
|
N817.py:10:26: N817 CamelCase `ElementTree` imported as acronym `ET`
|
9 | # Always an error (relative import)
10 | from ..xml.eltree import ElementTree as ET
| ^^^^^^^^^^^^^^^^^ N817
|

View File

@@ -15,21 +15,29 @@ use crate::settings::types::PythonVersion;
/// Exception handling via `try`-`except` blocks incurs some performance
/// overhead, regardless of whether an exception is raised.
///
/// When possible, refactor your code to put the entire loop into the
/// `try`-`except` block, rather than wrapping each iteration in a separate
/// `try`-`except` block.
/// To optimize your code, two techniques are possible:
/// 1. Refactor your code to put the entire loop into the `try`-`except` block,
/// rather than wrapping each iteration in a separate `try`-`except` block.
/// 2. Use "Look Before You Leap" idioms that attempt to avoid exceptions
/// being raised in the first place, avoiding the need to use `try`-`except`
/// blocks in the first place.
///
/// This rule is only enforced for Python versions prior to 3.11, which
/// introduced "zero cost" exception handling.
/// introduced "zero-cost" exception handling. However, note that even on
/// Python 3.11 and newer, refactoring your code to avoid exception handling in
/// tight loops can provide a significant speedup in some cases, as zero-cost
/// exception handling is only zero-cost in the "happy path" where no exception
/// is raised in the `try`-`except` block.
///
/// Note that, as with all `perflint` rules, this is only intended as a
/// micro-optimization, and will have a negligible impact on performance in
/// most cases.
/// As with all `perflint` rules, this is only intended as a
/// micro-optimization. In many cases, it will have a negligible impact on
/// performance.
///
/// ## Example
/// ```python
/// string_numbers: list[str] = ["1", "2", "three", "4", "5"]
///
/// # `try`/`except` that could be moved out of the loop:
/// int_numbers: list[int] = []
/// for num in string_numbers:
/// try:
@@ -37,6 +45,16 @@ use crate::settings::types::PythonVersion;
/// except ValueError as e:
/// print(f"Couldn't convert to integer: {e}")
/// break
///
/// # `try`/`except` used when "look before you leap" idioms could be used:
/// number_names: dict[int, str] = {1: "one", 3: "three", 4: "four"}
/// for number in range(5):
/// try:
/// name = number_names[number]
/// except KeyError:
/// continue
/// else:
/// print(f"The name of {number} is {name}")
/// ```
///
/// Use instead:
@@ -49,6 +67,12 @@ use crate::settings::types::PythonVersion;
/// int_numbers.append(int(num))
/// except ValueError as e:
/// print(f"Couldn't convert to integer: {e}")
///
/// number_names: dict[int, str] = {1: "one", 3: "three", 4: "four"}
/// for number in range(5):
/// name = number_names.get(number)
/// if name is not None:
/// print(f"The name of {number} is {name}")
/// ```
///
/// ## Options

View File

@@ -34,6 +34,7 @@ use crate::registry::Rule;
///
/// ```python
/// class PhotoMetadata:
///
/// """Metadata about a photo."""
/// ```
///
@@ -125,6 +126,7 @@ impl AlwaysFixableViolation for OneBlankLineAfterClass {
///
/// ```python
/// class PhotoMetadata:
///
/// """Metadata about a photo."""
/// ```
///

View File

@@ -91,13 +91,18 @@ pub(crate) fn bad_staticmethod_argument(
return;
};
if matches!(self_or_cls.name.as_str(), "self" | "cls") {
let diagnostic = Diagnostic::new(
BadStaticmethodArgument {
argument_name: self_or_cls.name.to_string(),
},
self_or_cls.range(),
);
diagnostics.push(diagnostic);
match (name.as_str(), self_or_cls.name.as_str()) {
("__new__", "cls") => {
return;
}
(_, "self" | "cls") => {}
_ => return,
}
diagnostics.push(Diagnostic::new(
BadStaticmethodArgument {
argument_name: self_or_cls.name.to_string(),
},
self_or_cls.range(),
));
}

View File

@@ -8,7 +8,7 @@ self_or_cls_assignment.py:4:9: PLW0642 Reassigned `cls` variable in class method
4 | cls = "apple" # PLW0642
| ^^^ PLW0642
5 | cls: Fruit = "apple" # PLW0642
6 | cls += "orange" # PLW0642
6 | cls += "orange" # OK, augmented assignments are ignored
|
= help: Consider using a different variable name
@@ -18,26 +18,15 @@ self_or_cls_assignment.py:5:9: PLW0642 Reassigned `cls` variable in class method
4 | cls = "apple" # PLW0642
5 | cls: Fruit = "apple" # PLW0642
| ^^^ PLW0642
6 | cls += "orange" # PLW0642
6 | cls += "orange" # OK, augmented assignments are ignored
7 | *cls = "banana" # PLW0642
|
= help: Consider using a different variable name
self_or_cls_assignment.py:6:9: PLW0642 Reassigned `cls` variable in class method
|
4 | cls = "apple" # PLW0642
5 | cls: Fruit = "apple" # PLW0642
6 | cls += "orange" # PLW0642
| ^^^ PLW0642
7 | *cls = "banana" # PLW0642
8 | cls, blah = "apple", "orange" # PLW0642
|
= help: Consider using a different variable name
self_or_cls_assignment.py:7:10: PLW0642 Reassigned `cls` variable in class method
|
5 | cls: Fruit = "apple" # PLW0642
6 | cls += "orange" # PLW0642
6 | cls += "orange" # OK, augmented assignments are ignored
7 | *cls = "banana" # PLW0642
| ^^^ PLW0642
8 | cls, blah = "apple", "orange" # PLW0642
@@ -47,7 +36,7 @@ self_or_cls_assignment.py:7:10: PLW0642 Reassigned `cls` variable in class metho
self_or_cls_assignment.py:8:9: PLW0642 Reassigned `cls` variable in class method
|
6 | cls += "orange" # PLW0642
6 | cls += "orange" # OK, augmented assignments are ignored
7 | *cls = "banana" # PLW0642
8 | cls, blah = "apple", "orange" # PLW0642
| ^^^ PLW0642
@@ -94,7 +83,7 @@ self_or_cls_assignment.py:17:9: PLW0642 Reassigned `self` variable in instance m
17 | self = "red" # PLW0642
| ^^^^ PLW0642
18 | self: Self = "red" # PLW0642
19 | self += "blue" # PLW0642
19 | self += "blue" # OK, augmented assignments are ignored
|
= help: Consider using a different variable name
@@ -104,26 +93,15 @@ self_or_cls_assignment.py:18:9: PLW0642 Reassigned `self` variable in instance m
17 | self = "red" # PLW0642
18 | self: Self = "red" # PLW0642
| ^^^^ PLW0642
19 | self += "blue" # PLW0642
19 | self += "blue" # OK, augmented assignments are ignored
20 | *self = "blue" # PLW0642
|
= help: Consider using a different variable name
self_or_cls_assignment.py:19:9: PLW0642 Reassigned `self` variable in instance method
|
17 | self = "red" # PLW0642
18 | self: Self = "red" # PLW0642
19 | self += "blue" # PLW0642
| ^^^^ PLW0642
20 | *self = "blue" # PLW0642
21 | self, blah = "red", "blue" # PLW0642
|
= help: Consider using a different variable name
self_or_cls_assignment.py:20:10: PLW0642 Reassigned `self` variable in instance method
|
18 | self: Self = "red" # PLW0642
19 | self += "blue" # PLW0642
19 | self += "blue" # OK, augmented assignments are ignored
20 | *self = "blue" # PLW0642
| ^^^^ PLW0642
21 | self, blah = "red", "blue" # PLW0642
@@ -133,7 +111,7 @@ self_or_cls_assignment.py:20:10: PLW0642 Reassigned `self` variable in instance
self_or_cls_assignment.py:21:9: PLW0642 Reassigned `self` variable in instance method
|
19 | self += "blue" # PLW0642
19 | self += "blue" # OK, augmented assignments are ignored
20 | *self = "blue" # PLW0642
21 | self, blah = "red", "blue" # PLW0642
| ^^^^ PLW0642

View File

@@ -15,7 +15,7 @@ FURB177.py:5:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for curr
3 3 |
4 4 | # Errors
5 |-_ = Path().resolve()
5 |+_ = pathlib.Path.cwd()
5 |+_ = Path.cwd()
6 6 | _ = pathlib.Path().resolve()
7 7 |
8 8 | _ = Path("").resolve()
@@ -36,7 +36,7 @@ FURB177.py:6:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for curr
4 4 | # Errors
5 5 | _ = Path().resolve()
6 |-_ = pathlib.Path().resolve()
6 |+_ = pathlib.Path.cwd()
6 |+_ = Path.cwd()
7 7 |
8 8 | _ = Path("").resolve()
9 9 | _ = pathlib.Path("").resolve()
@@ -56,7 +56,7 @@ FURB177.py:8:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for curr
6 6 | _ = pathlib.Path().resolve()
7 7 |
8 |-_ = Path("").resolve()
8 |+_ = pathlib.Path.cwd()
8 |+_ = Path.cwd()
9 9 | _ = pathlib.Path("").resolve()
10 10 |
11 11 | _ = Path(".").resolve()
@@ -76,7 +76,7 @@ FURB177.py:9:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for curr
7 7 |
8 8 | _ = Path("").resolve()
9 |-_ = pathlib.Path("").resolve()
9 |+_ = pathlib.Path.cwd()
9 |+_ = Path.cwd()
10 10 |
11 11 | _ = Path(".").resolve()
12 12 | _ = pathlib.Path(".").resolve()
@@ -96,7 +96,7 @@ FURB177.py:11:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for cur
9 9 | _ = pathlib.Path("").resolve()
10 10 |
11 |-_ = Path(".").resolve()
11 |+_ = pathlib.Path.cwd()
11 |+_ = Path.cwd()
12 12 | _ = pathlib.Path(".").resolve()
13 13 |
14 14 | _ = Path("", **kwargs).resolve()
@@ -116,7 +116,7 @@ FURB177.py:12:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for cur
10 10 |
11 11 | _ = Path(".").resolve()
12 |-_ = pathlib.Path(".").resolve()
12 |+_ = pathlib.Path.cwd()
12 |+_ = Path.cwd()
13 13 |
14 14 | _ = Path("", **kwargs).resolve()
15 15 | _ = pathlib.Path("", **kwargs).resolve()
@@ -136,7 +136,7 @@ FURB177.py:14:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for cur
12 12 | _ = pathlib.Path(".").resolve()
13 13 |
14 |-_ = Path("", **kwargs).resolve()
14 |+_ = pathlib.Path.cwd()
14 |+_ = Path.cwd()
15 15 | _ = pathlib.Path("", **kwargs).resolve()
16 16 |
17 17 | _ = Path(".", **kwargs).resolve()
@@ -156,7 +156,7 @@ FURB177.py:15:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for cur
13 13 |
14 14 | _ = Path("", **kwargs).resolve()
15 |-_ = pathlib.Path("", **kwargs).resolve()
15 |+_ = pathlib.Path.cwd()
15 |+_ = Path.cwd()
16 16 |
17 17 | _ = Path(".", **kwargs).resolve()
18 18 | _ = pathlib.Path(".", **kwargs).resolve()
@@ -176,7 +176,7 @@ FURB177.py:17:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for cur
15 15 | _ = pathlib.Path("", **kwargs).resolve()
16 16 |
17 |-_ = Path(".", **kwargs).resolve()
17 |+_ = pathlib.Path.cwd()
17 |+_ = Path.cwd()
18 18 | _ = pathlib.Path(".", **kwargs).resolve()
19 19 |
20 20 | # OK
@@ -196,7 +196,7 @@ FURB177.py:18:5: FURB177 [*] Prefer `Path.cwd()` over `Path().resolve()` for cur
16 16 |
17 17 | _ = Path(".", **kwargs).resolve()
18 |-_ = pathlib.Path(".", **kwargs).resolve()
18 |+_ = pathlib.Path.cwd()
18 |+_ = Path.cwd()
19 19 |
20 20 | # OK
21 21 | _ = Path.cwd()

View File

@@ -1,10 +1,10 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast};
use ruff_python_ast as ast;
use ruff_python_literal::format::FormatSpec;
use ruff_python_parser::parse_expression;
use ruff_python_semantic::analyze::logging;
use ruff_python_semantic::SemanticModel;
use ruff_python_semantic::analyze::logging::is_logger_candidate;
use ruff_python_semantic::{Modules, SemanticModel};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};
@@ -12,6 +12,7 @@ use memchr::memchr2_iter;
use rustc_hash::FxHashSet;
use crate::checkers::ast::Checker;
use crate::rules::fastapi::rules::is_fastapi_route_call;
/// ## What it does
/// Searches for strings that look like they were meant to be f-strings, but are missing an `f` prefix.
@@ -33,6 +34,8 @@ use crate::checkers::ast::Checker;
/// 4. The string has no `{...}` expression sections, or uses invalid f-string syntax.
/// 5. The string references variables that are not in scope, or it doesn't capture variables at all.
/// 6. Any format specifiers in the potential f-string are invalid.
/// 7. The string is part of a function call that is known to expect a template string rather than an
/// evaluated f-string: for example, a [`logging`] call, a [`gettext`] call, or a [`fastAPI` path].
///
/// ## Example
///
@@ -48,6 +51,10 @@ use crate::checkers::ast::Checker;
/// day_of_week = "Tuesday"
/// print(f"Hello {name}! It is {day_of_week} today!")
/// ```
///
/// [`logging`]: https://docs.python.org/3/howto/logging-cookbook.html#using-particular-formatting-styles-throughout-your-application
/// [`gettext`]: https://docs.python.org/3/library/gettext.html
/// [`fastAPI` path]: https://fastapi.tiangolo.com/tutorial/path-params/
#[violation]
pub struct MissingFStringSyntax;
@@ -75,11 +82,25 @@ pub(crate) fn missing_fstring_syntax(checker: &mut Checker, literal: &ast::Strin
}
}
// We also want to avoid expressions that are intended to be translated.
if semantic.current_expressions().any(|expr| {
is_gettext(expr, semantic)
|| is_logger_call(expr, semantic, &checker.settings.logger_objects)
}) {
let logger_objects = &checker.settings.logger_objects;
let fastapi_seen = semantic.seen_module(Modules::FASTAPI);
// We also want to avoid:
// - Expressions inside `gettext()` calls
// - Expressions passed to logging calls (since the `logging` module evaluates them lazily:
// https://docs.python.org/3/howto/logging-cookbook.html#using-particular-formatting-styles-throughout-your-application)
// - `fastAPI` paths: https://fastapi.tiangolo.com/tutorial/path-params/
// - Expressions where a method is immediately called on the string literal
if semantic
.current_expressions()
.filter_map(ast::Expr::as_call_expr)
.any(|call_expr| {
is_method_call_on_literal(call_expr, literal)
|| is_gettext(call_expr, semantic)
|| is_logger_candidate(&call_expr.func, semantic, logger_objects)
|| (fastapi_seen && is_fastapi_route_call(call_expr, semantic))
})
{
return;
}
@@ -90,13 +111,6 @@ pub(crate) fn missing_fstring_syntax(checker: &mut Checker, literal: &ast::Strin
}
}
fn is_logger_call(expr: &ast::Expr, semantic: &SemanticModel, logger_objects: &[String]) -> bool {
let ast::Expr::Call(ast::ExprCall { func, .. }) = expr else {
return false;
};
logging::is_logger_candidate(func, semantic, logger_objects)
}
/// Returns `true` if an expression appears to be a `gettext` call.
///
/// We want to avoid statement expressions and assignments related to aliases
@@ -107,12 +121,9 @@ fn is_logger_call(expr: &ast::Expr, semantic: &SemanticModel, logger_objects: &[
/// and replace the original string with its translated counterpart. If the
/// string contains variable placeholders or formatting, it can complicate the
/// translation process, lead to errors or incorrect translations.
fn is_gettext(expr: &ast::Expr, semantic: &SemanticModel) -> bool {
let ast::Expr::Call(ast::ExprCall { func, .. }) = expr else {
return false;
};
let short_circuit = match func.as_ref() {
fn is_gettext(call_expr: &ast::ExprCall, semantic: &SemanticModel) -> bool {
let func = &*call_expr.func;
let short_circuit = match func {
ast::Expr::Name(ast::ExprName { id, .. }) => {
matches!(id.as_str(), "gettext" | "ngettext" | "_")
}
@@ -136,6 +147,21 @@ fn is_gettext(expr: &ast::Expr, semantic: &SemanticModel) -> bool {
})
}
/// Return `true` if `call_expr` is a method call on an [`ast::ExprStringLiteral`]
/// in which `literal` is one of the [`ast::StringLiteral`] parts.
///
/// For example: `expr` is a node representing the expression `"{foo}".format(foo="bar")`,
/// and `literal` is the node representing the string literal `"{foo}"`.
fn is_method_call_on_literal(call_expr: &ast::ExprCall, literal: &ast::StringLiteral) -> bool {
let ast::Expr::Attribute(ast::ExprAttribute { value, .. }) = &*call_expr.func else {
return false;
};
let ast::Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &**value else {
return false;
};
value.as_slice().contains(literal)
}
/// Returns `true` if `literal` is likely an f-string with a missing `f` prefix.
/// See [`MissingFStringSyntax`] for the validation criteria.
fn should_be_fstring(
@@ -158,55 +184,28 @@ fn should_be_fstring(
};
let mut arg_names = FxHashSet::default();
let mut last_expr: Option<&ast::Expr> = None;
for expr in semantic.current_expressions() {
match expr {
ast::Expr::Call(ast::ExprCall {
arguments: ast::Arguments { keywords, args, .. },
func,
..
}) => {
if let ast::Expr::Attribute(ast::ExprAttribute { value, .. }) = func.as_ref() {
match value.as_ref() {
// if the first part of the attribute is the string literal,
// we want to ignore this literal from the lint.
// for example: `"{x}".some_method(...)`
ast::Expr::StringLiteral(expr_literal)
if expr_literal.value.as_slice().contains(literal) =>
{
return false;
}
// if the first part of the attribute was the expression we
// just went over in the last iteration, then we also want to pass
// this over in the lint.
// for example: `some_func("{x}").some_method(...)`
value if last_expr == Some(value) => {
return false;
}
_ => {}
}
}
for keyword in &**keywords {
if let Some(ident) = keyword.arg.as_ref() {
arg_names.insert(ident.as_str());
}
}
for arg in &**args {
if let ast::Expr::Name(ast::ExprName { id, .. }) = arg {
arg_names.insert(id.as_str());
}
}
for expr in semantic
.current_expressions()
.filter_map(ast::Expr::as_call_expr)
{
let ast::Arguments { keywords, args, .. } = &expr.arguments;
for keyword in &**keywords {
if let Some(ident) = keyword.arg.as_ref() {
arg_names.insert(&ident.id);
}
}
for arg in &**args {
if let ast::Expr::Name(ast::ExprName { id, .. }) = arg {
arg_names.insert(id);
}
_ => continue,
}
last_expr.replace(expr);
}
for f_string in value.f_strings() {
let mut has_name = false;
for element in f_string.elements.expressions() {
if let ast::Expr::Name(ast::ExprName { id, .. }) = element.expression.as_ref() {
if arg_names.contains(id.as_str()) {
if arg_names.contains(id) {
return false;
}
if semantic

View File

@@ -4,8 +4,10 @@ use ruff_python_ast::identifier::Identifier;
use ruff_python_ast::visitor::source_order;
use ruff_python_ast::{self as ast, AnyNodeRef, Expr, Stmt};
use ruff_python_semantic::analyze::function_type::is_stub;
use ruff_python_semantic::Modules;
use crate::checkers::ast::Checker;
use crate::rules::fastapi::rules::is_fastapi_route;
/// ## What it does
/// Checks for functions declared `async` that do not await or otherwise use features requiring the
@@ -173,6 +175,12 @@ pub(crate) fn unused_async(
return;
}
if checker.semantic().seen_module(Modules::FASTAPI)
&& is_fastapi_route(function_def, checker.semantic())
{
return;
}
let found_await_or_async = {
let mut visitor = AsyncExprVisitor::default();
source_order::walk_body(&mut visitor, body);

View File

@@ -907,7 +907,7 @@ impl<'a> SemanticModel<'a> {
self.current_scopes()
.enumerate()
.find_map(|(scope_index, scope)| {
scope.bindings().find_map(|(name, binding_id)| {
let mut imported_names = scope.bindings().filter_map(|(name, binding_id)| {
let binding = &self.bindings[binding_id];
match &binding.kind {
// Ex) Given `module="sys"` and `object="exit"`:
@@ -987,7 +987,22 @@ impl<'a> SemanticModel<'a> {
_ => {}
}
None
})
});
let first = imported_names.next()?;
if let Some(second) = imported_names.next() {
// Multiple candidates. We need to sort them because `scope.bindings()` is a HashMap
// which doesn't have a stable iteration order.
let mut imports: Vec<_> =
[first, second].into_iter().chain(imported_names).collect();
imports.sort_unstable_by_key(|import| import.range.start());
// Return the binding that was imported last.
imports.pop()
} else {
Some(first)
}
})
}

View File

@@ -123,11 +123,7 @@ pub(crate) fn fix_all(
fixes.insert(
url.clone(),
vec![lsp_types::TextEdit {
range: source_range.to_range(
source_kind.source_code(),
&source_index,
encoding,
),
range: source_range.to_range(&source, &source_index, encoding),
new_text: modified[modified_range].to_owned(),
}],
);

View File

@@ -1,6 +1,6 @@
[package]
name = "ruff_wasm"
version = "0.6.0"
version = "0.6.1"
publish = false
authors = { workspace = true }
edition = { workspace = true }

View File

@@ -78,7 +78,7 @@ Ruff can be used as a [pre-commit](https://pre-commit.com) hook via [`ruff-pre-c
```yaml
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.6.0
rev: v0.6.1
hooks:
# Run the linter.
- id: ruff
@@ -91,7 +91,7 @@ To enable lint fixes, add the `--fix` argument to the lint hook:
```yaml
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.6.0
rev: v0.6.1
hooks:
# Run the linter.
- id: ruff
@@ -105,7 +105,7 @@ To run the hooks over Jupyter Notebooks too, add `jupyter` to the list of allowe
```yaml
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.6.0
rev: v0.6.1
hooks:
# Run the linter.
- id: ruff

View File

@@ -61,6 +61,9 @@ markdown_extensions:
plugins:
- search
- typeset
- mike:
deploy_prefix: site/ruff
canonical_version: latest
extra_css:
- stylesheets/extra.css
extra_javascript:
@@ -71,3 +74,6 @@ not_in_nav: |
extra:
analytics:
provider: fathom
version:
provider: mike
alias: true

View File

@@ -4,7 +4,7 @@ build-backend = "maturin"
[project]
name = "ruff"
version = "0.6.0"
version = "0.6.1"
description = "An extremely fast Python linter and code formatter, written in Rust."
authors = [{ name = "Astral Software Inc.", email = "hey@astral.sh" }]
readme = "README.md"

1
ruff.schema.json generated
View File

@@ -3121,6 +3121,7 @@
"FAST00",
"FAST001",
"FAST002",
"FAST003",
"FBT",
"FBT0",
"FBT00",

View File

@@ -1,6 +1,6 @@
[tool.poetry]
name = "scripts"
version = "0.6.0"
version = "0.6.1"
description = ""
authors = ["Charles Marsh <charlie.r.marsh@gmail.com>"]

View File

@@ -34,6 +34,7 @@ KNOWN_FORMATTING_VIOLATIONS = [
"bad-quotes-inline-string",
"bad-quotes-multiline-string",
"blank-line-after-decorator",
"blank-line-before-class",
"blank-line-between-methods",
"blank-lines-after-function-or-class",
"blank-lines-before-nested-definition",
@@ -67,6 +68,7 @@ KNOWN_FORMATTING_VIOLATIONS = [
"no-space-after-inline-comment",
"non-empty-stub-body",
"one-blank-line-after-class",
"one-blank-line-before-class",
"over-indentation",
"over-indented",
"pass-statement-stub-body",