Skip to content

Commit

Permalink
Merge branch 'main' into dcreager/terminal-visibility
Browse files Browse the repository at this point in the history
* main:
  [red-knot] Extend instance-attribute tests (#15808)
  Fix formatter warning message for `flake8-quotes` option (#15788)
  [`flake8-bugbear`] Exempt `NewType` calls where the original type is immutable (`B008`) (#15765)
  Add missing config docstrings (#15803)
  [`refurb`] Do not emit diagnostic when loop variables are used outside loop body (`FURB122`) (#15757)
  [`ruff`] Check for shadowed `map` before suggesting fix (`RUF058`) (#15790)
  [red-knot] Do not use explicit `knot_extensions.Unknown` declaration (#15787)
  Preserve quotes in generated byte strings (#15778)
  [minor] Simplify some `ExprStringLiteral` creation logic (#15775)
  Preserve quote style in generated code (#15726)
  Rename internal helper functions (#15771)
  [`airflow`] Extend airflow context parameter check for `BaseOperator.execute` (`AIR302`) (#15713)
  Implement tab autocomplete for `ruff config` (#15603)
  • Loading branch information
dcreager committed Jan 29, 2025
2 parents 32e8131 + 0f1035b commit 2932bb4
Show file tree
Hide file tree
Showing 54 changed files with 1,974 additions and 997 deletions.
198 changes: 135 additions & 63 deletions crates/red_knot_python_semantic/resources/mdtest/attributes.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,97 +13,115 @@ accessed on the class itself.

```py
class C:
def __init__(self, value2: int, flag: bool = False) -> None:
# bound but not declared
self.pure_instance_variable1 = "value set in __init__"

# bound but not declared - with type inferred from parameter
self.pure_instance_variable2 = value2

# declared but not bound
self.pure_instance_variable3: bytes

# declared and bound
self.pure_instance_variable4: bool = True

# possibly undeclared/unbound
def __init__(self, param: int | None, flag: bool = False) -> None:
value = 1 if flag else "a"
self.inferred_from_value = value
self.inferred_from_other_attribute = self.inferred_from_value
self.inferred_from_param = param
self.declared_only: bytes
self.declared_and_bound: bool = True
if flag:
self.pure_instance_variable5: str = "possibly set in __init__"
self.possibly_undeclared_unbound: str = "possibly set in __init__"

c_instance = C(1)

# TODO: should be `Literal["value set in __init__"]`, or `Unknown | Literal[…]` to allow
# assignments to this unannotated attribute from other scopes.
reveal_type(c_instance.pure_instance_variable1) # revealed: @Todo(implicit instance attribute)
# TODO: Mypy/pyright infer `int | str` here. We want this to be `Unknown | Literal[1, "a"]`
reveal_type(c_instance.inferred_from_value) # revealed: @Todo(implicit instance attribute)

# TODO: should be `int`
reveal_type(c_instance.pure_instance_variable2) # revealed: @Todo(implicit instance attribute)
# TODO: Same here. This should be `Unknown | Literal[1, "a"]`
reveal_type(c_instance.inferred_from_other_attribute) # revealed: @Todo(implicit instance attribute)

# TODO: should be `int | None`
reveal_type(c_instance.inferred_from_param) # revealed: @Todo(implicit instance attribute)

# TODO: should be `bytes`
reveal_type(c_instance.pure_instance_variable3) # revealed: @Todo(implicit instance attribute)
reveal_type(c_instance.declared_only) # revealed: @Todo(implicit instance attribute)

# TODO: should be `bool`
reveal_type(c_instance.pure_instance_variable4) # revealed: @Todo(implicit instance attribute)
reveal_type(c_instance.declared_and_bound) # revealed: @Todo(implicit instance attribute)

# TODO: should be `str`
# We probably don't want to emit a diagnostic for this being possibly undeclared/unbound.
# mypy and pyright do not show an error here.
reveal_type(c_instance.pure_instance_variable5) # revealed: @Todo(implicit instance attribute)
reveal_type(c_instance.possibly_undeclared_unbound) # revealed: @Todo(implicit instance attribute)

# This assignment is fine, as we infer `Unknown | Literal[1, "a"]` for `inferred_from_value`.
c_instance.inferred_from_value = "value set on instance"

# TODO: If we choose to infer a precise `Literal[…]` type for the instance attribute (see
# above), this should be an error: incompatible types in assignment. If we choose to infer
# a gradual `Unknown | Literal[…]` type, this assignment is fine.
c_instance.pure_instance_variable1 = "value set on instance"
# This assignment is also fine:
c_instance.inferred_from_param = None

# TODO: this should be an error (incompatible types in assignment)
c_instance.pure_instance_variable2 = "incompatible"
c_instance.inferred_from_param = "incompatible"

# TODO: we already show an error here but the message might be improved?
# mypy shows no error here, but pyright raises "reportAttributeAccessIssue"
# error: [unresolved-attribute] "Type `Literal[C]` has no attribute `pure_instance_variable1`"
reveal_type(C.pure_instance_variable1) # revealed: Unknown
# error: [unresolved-attribute] "Type `Literal[C]` has no attribute `inferred_from_value`"
reveal_type(C.inferred_from_value) # revealed: Unknown

# TODO: this should be an error (pure instance variables cannot be accessed on the class)
# mypy shows no error here, but pyright raises "reportAttributeAccessIssue"
C.pure_instance_variable1 = "overwritten on class"
C.inferred_from_value = "overwritten on class"

c_instance.pure_instance_variable4 = False
# This assignment is fine:
c_instance.declared_and_bound = False

# TODO: After this assignment to the attribute within this scope, we may eventually want to narrow
# the `bool` type (see above) for this instance variable to `Literal[False]` here. This is unsound
# in general (we don't know what else happened to `c_instance` between the assignment and the use
# here), but mypy and pyright support this. In conclusion, this could be `bool` but should probably
# be `Literal[False]`.
reveal_type(c_instance.pure_instance_variable4) # revealed: @Todo(implicit instance attribute)
reveal_type(c_instance.declared_and_bound) # revealed: @Todo(implicit instance attribute)
```

#### Variable declared in class body and declared/bound in `__init__`
#### Variable declared in class body and possibly bound in `__init__`

The same rule applies even if the variable is *declared* (not bound!) in the class body: it is still
a pure instance variable.

```py
class C:
pure_instance_variable: str
declared_and_bound: str | None

def __init__(self) -> None:
self.pure_instance_variable = "value set in __init__"
self.declared_and_bound = "value set in __init__"

c_instance = C()

reveal_type(c_instance.pure_instance_variable) # revealed: str
reveal_type(c_instance.declared_and_bound) # revealed: str | None

# TODO: we currently plan to emit a diagnostic here. Note that both mypy
# and pyright show no error in this case! So we may reconsider this in
# the future, if it turns out to produce too many false positives.
reveal_type(C.pure_instance_variable) # revealed: str
reveal_type(C.declared_and_bound) # revealed: str | None

# TODO: same as above. We plan to emit a diagnostic here, even if both mypy
# and pyright allow this.
C.pure_instance_variable = "overwritten on class"
C.declared_and_bound = "overwritten on class"

# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `declared_and_bound` of type `str | None`"
c_instance.declared_and_bound = 1
```

#### Variable declared in class body and not bound anywhere

If a variable is declared in the class body but not bound anywhere, we still consider it a pure
instance variable and allow access to it via instances.

```py
class C:
only_declared: str

c_instance = C()

reveal_type(c_instance.only_declared) # revealed: str

# TODO: mypy and pyright do not show an error here, but we plan to emit a diagnostic.
# The type could be changed to 'Unknown' if we decide to emit an error?
reveal_type(C.only_declared) # revealed: str

# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `pure_instance_variable` of type `str`"
c_instance.pure_instance_variable = 1
# TODO: mypy and pyright do not show an error here, but we plan to emit one.
C.only_declared = "overwritten on class"
```

#### Variable only defined in unrelated method
Expand All @@ -112,45 +130,66 @@ We also recognize pure instance variables if they are defined in a method that i

```py
class C:
def set_instance_variable(self) -> None:
self.pure_instance_variable = "value set in method"
def __init__(self, param: int | None, flag: bool = False) -> None:
self.initialize(param, flag)

c_instance = C()
def initialize(self, param: int | None, flag: bool) -> None:
value = 1 if flag else "a"
self.inferred_from_value = value
self.inferred_from_other_attribute = self.inferred_from_value
self.inferred_from_param = param
self.declared_only: bytes
self.declared_and_bound: bool = True

c_instance = C(1)

# TODO: Should be `Unknown | Literal[1, "a"]`
reveal_type(c_instance.inferred_from_value) # revealed: @Todo(implicit instance attribute)

# TODO: Should be `Unknown | Literal[1, "a"]`
reveal_type(c_instance.inferred_from_other_attribute) # revealed: @Todo(implicit instance attribute)

# Not that we would use this in static analysis, but for a more realistic example, let's actually
# call the method, so that the attribute is bound if this example is actually run.
c_instance.set_instance_variable()
# TODO: Should be `int | None`
reveal_type(c_instance.inferred_from_param) # revealed: @Todo(implicit instance attribute)

# TODO: should be `Literal["value set in method"]` or `Unknown | Literal[…]` (see above).
reveal_type(c_instance.pure_instance_variable) # revealed: @Todo(implicit instance attribute)
# TODO: Should be `bytes`
reveal_type(c_instance.declared_only) # revealed: @Todo(implicit instance attribute)

# TODO: Should be `bool`
reveal_type(c_instance.declared_and_bound) # revealed: @Todo(implicit instance attribute)

# TODO: We already show an error here, but the message might be improved?
# error: [unresolved-attribute]
reveal_type(C.pure_instance_variable) # revealed: Unknown
reveal_type(C.inferred_from_value) # revealed: Unknown

# TODO: this should be an error
C.pure_instance_variable = "overwritten on class"
C.inferred_from_value = "overwritten on class"
```

#### Variable declared in class body and not bound anywhere

If a variable is declared in the class body but not bound anywhere, we still consider it a pure
instance variable and allow access to it via instances.
#### Methods that does not use `self` as a first parameter

```py
class C:
pure_instance_variable: str
# This might trigger a stylistic lint like `invalid-first-argument-name-for-method`, but
# it should be supported in general:
def __init__(this) -> None:
this.declared_and_bound: str | None = "a"

c_instance = C()
# TODO: should be `str | None`
reveal_type(C().declared_and_bound) # revealed: @Todo(implicit instance attribute)
```

reveal_type(c_instance.pure_instance_variable) # revealed: str
#### Aliased `self` parameter

# TODO: mypy and pyright do not show an error here, but we plan to emit a diagnostic.
# The type could be changed to 'Unknown' if we decide to emit an error?
reveal_type(C.pure_instance_variable) # revealed: str
```py
class C:
def __init__(self) -> None:
this = self
this.declared_and_bound: str | None = "a"

# TODO: mypy and pyright do not show an error here, but we plan to emit one.
C.pure_instance_variable = "overwritten on class"
# TODO: This would ideally be `str | None`, but mypy/pyright don't support this either,
# so `Unknown` + a diagnostic is also fine.
reveal_type(C().declared_and_bound) # revealed: @Todo(implicit instance attribute)
```

### Pure class variables (`ClassVar`)
Expand Down Expand Up @@ -277,6 +316,39 @@ reveal_type(C.variable_with_class_default1) # revealed: str
reveal_type(c_instance.variable_with_class_default1) # revealed: str
```

### Inheritance of class/instance attributes

#### Instance variable defined in a base class

```py
class Base:
declared_in_body: int | None = 1

can_not_be_redeclared: str | None = None

def __init__(self) -> None:
self.defined_in_init: str | None = "value in base"

class Intermediate(Base):
# TODO: Mypy does not report an error here, but pyright does:
# "… overrides symbol of same name in class "Base". Variable is mutable so its type is invariant"
# We should introduce a diagnostic for this. Whether or not that should be enabled by default can
# still be discussed.
can_not_be_redeclared: str = "a"

def __init__(self) -> None:
super().__init__()

class Derived(Intermediate): ...

reveal_type(Derived.declared_in_body) # revealed: int | None

reveal_type(Derived().declared_in_body) # revealed: int | None

# TODO: Should be `str | None`
reveal_type(Derived().defined_in_init) # revealed: @Todo(implicit instance attribute)
```

## Union of attributes

```py
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,17 +202,16 @@ a = None

### Undeclared but bound

If a symbols is undeclared, we use the union of `Unknown` with the inferred type. Note that we treat
symbols that are undeclared differently from symbols that are explicitly declared as `Unknown`:
If a symbol is *undeclared*, we use the union of `Unknown` with the inferred type. Note that we
treat this case differently from the case where a symbol is implicitly declared with `Unknown`,
possibly due to the usage of an unknown name in the annotation:

```py path=mod.py
from knot_extensions import Unknown

# Undeclared:
a = 1

# Declared with `Unknown`:
b: Unknown = 1
# Implicitly declared with `Unknown`, due to the usage of an unknown name in the annotation:
b: SomeUnknownName = 1 # error: [unresolved-reference]
```

```py
Expand All @@ -231,13 +230,11 @@ If a symbol is undeclared and *possibly* unbound, we currently do not raise an e
inconsistent when compared to the "possibly-undeclared-and-possibly-unbound" case.

```py path=mod.py
from knot_extensions import Unknown

def flag() -> bool: ...

if flag:
a = 1
b: Unknown = 1
b: SomeUnknownName = 1 # error: [unresolved-reference]
```

```py
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ cargo test -p red_knot_python_semantic -- mdtest__
Alternatively, you can use the `mdtest.py` runner which has a watch mode that will re-run corresponding tests when Markdown files change, and recompile automatically when Rust code changes:

```bash
uv -q run crates/red_knot_python_semantic/mdtest.py
uv run crates/red_knot_python_semantic/mdtest.py
```

## Planned features
Expand Down
8 changes: 7 additions & 1 deletion crates/ruff/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ use ruff_workspace::resolver::ConfigurationTransformer;
use rustc_hash::FxHashMap;
use toml;

use crate::commands::completions::config::{OptionString, OptionStringParser};

/// All configuration options that can be passed "globally",
/// i.e., can be passed to all subcommands
#[derive(Debug, Default, Clone, clap::Args)]
Expand Down Expand Up @@ -114,7 +116,11 @@ pub enum Command {
/// List or describe the available configuration options.
Config {
/// Config key to show
option: Option<String>,
#[arg(
value_parser = OptionStringParser,
hide_possible_values = true
)]
option: Option<OptionString>,
/// Output format
#[arg(long, value_enum, default_value = "text")]
output_format: HelpFormat,
Expand Down
Loading

0 comments on commit 2932bb4

Please sign in to comment.