Skip to content

type-statement RHS validation is substring matching on source text, so most invalid type expressions pass silently #379

Description

@MelbourneDeveloper

Summary

aliases_type_statement validates the RHS of a PEP 695 type X = ... statement by substring-matching the raw source text, not by walking the Ruff AST. Almost every invalid type expression that does not start with one of a handful of hard-coded prefixes passes silently.

Reported on X: https://x.com/cyanchanges/status/2083115048143364512

Reported from the web playground; reproduced identically on the CLI, so this is not a WASM-only difference. Reproduced on a clean cargo build --release of main @ 5b756d3.

Reported reproduction

type c = "the" + list["of genshin"].impact.updates.that.you.should.definitely["try"]. \
  because.this["is not"].a.real.type.checker.wtf.ls["this"]

Basilisk: All checked. No issues found.

Note there are zero undefined names in that file — everything after list is attribute access — so unresolved-reference diagnostics cannot catch it either. The type-expression validator is the only thing that could, and it does not run.

What should error

Per the typing spec, the RHS of a type statement is a type expression, not an arbitrary expression:

  • "the" + list[...]+ is not a legal type-expression operator (only | is).
  • list["of genshin"] — the string is a forward reference, and of genshin is not parseable as an expression. Same for "is not" and "try" (a keyword).
  • .impact.updates.that… — member access on a subscript result is not a valid type-expression form. A qualified name must be mod.Name, never Sub[...].attr.

Minimal cases

type A = "the" + "thing"      # SILENT — should error (BinOp `+`)
type B = list["of genshin"]   # SILENT — should error (unparseable forward reference)
type D = list[int].attr       # SILENT — should error (attribute access on a subscript)
type E = 1 + 2                # caught

Only E fires — and only because the RHS text happens to begin with an ASCII digit.

Mechanism

crates/basilisk-checker/src/rules/aliases_type_statement.rs:44:

fn is_invalid_rhs(rhs: &str) -> bool {
    let rhs = rhs.trim();
    if rhs == "True" || rhs == "False" { return true; }
    if rhs.chars().next().is_some_and(|c| c.is_ascii_digit()) { return true; }
    if rhs.starts_with("f\"") || rhs.starts_with("f'") { return true; }
    if rhs.starts_with('[') { return true; }
    if rhs.starts_with('{') { return true; }
    if rhs.starts_with('(') && paren_has_top_level_comma(rhs) { return true; }
    if has_top_level_token(rhs, " if ") { return true; }
    if has_top_level_token(rhs, " or ") || has_top_level_token(rhs, " and ") { return true; }
    if rhs.contains("lambda") { return true; }
    if rhs.starts_with("eval(") { return true; }
    false
}

This is an allow-by-default list of textual shapes. The reported RHS begins with " (a string), matches no branch, and is accepted. It also means type X = lambda_case (a name containing the substring lambda) is a false positive waiting to happen, and type X = a if_b style text can be misread, because the scanner cannot distinguish code from string content.

Expected

Validate the StmtTypeAlias value node against the type-expression grammar on the Ruff AST:

  • allowed: Name, dotted Attribute chains rooted at a module/name, Subscript of an allowed base, BinOp(|), string forward references that themselves parse as valid type expressions, None, ... where the form permits it.
  • rejected: every other operator, call expressions (outside the sanctioned special forms), comparisons, comprehensions, literal displays, attribute access on a Subscript, and forward-reference strings that fail to parse.

This is exactly the migration that CHECKER-ELIMINATE-LINE-SCANNING-PLAN.md exists for, and it matches the repo rule "avoid regex to parse anything, use ruff". aliases_type_statement.rs is not currently in that plan's inventory; it should be, and it should be validated eagerly at binding time — PEP 695 lazy evaluation defers name resolution, never well-formedness of the expression.

Note on eager vs. lazy

Whether the alias is ever referenced must not matter. Nothing in the reported file references c, and the file must still be rejected.

Metadata

Metadata

Labels

criticalCritical severity — fix immediately

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions