Skip to content

--report-glean fix: Name methods on Literal and LiteralString receivers by their class #5605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Quaden2307 wants to merge 3 commits into from
Closed

--report-glean fix: Name methods on Literal and LiteralString receivers by their class #5605

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
60f5be8
Add glean snapshot for methods called on Literal str
Quaden2307 Nov 11, 2026
cbb3081
Name methods called on a Literal after the literal's class in glean
Quaden2307 Nov 11, 2026
1ab5b11
Use a single match in defining_type to improve readability
Quaden2307 Nov 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions pyrefly/lib/report/glean.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,28 @@ class Outer:
totals = [n * 2 for n in range(3)]
"#;

let string_literal = r#"from enum import StrEnum
from typing import Literal, LiteralString

def plain(word: str) -> str:
return word.title()

def literal(word: Literal["red"]) -> str:
return word.title()

def literal_string(word: LiteralString) -> str:
return word.title()

def joined(parts: list[str]) -> str:
return ", ".join(parts)

class Colour(StrEnum):
RED = "red"

def label(self) -> str:
return self.value.title()
"#;

let files = [
("simple", simple_code),
("classes", classes_code),
Expand All @@ -307,6 +329,7 @@ totals = [n * 2 for n in range(3)]
("inherited_members", inherited_members),
("nested_later_class", nested_later_class),
("comprehensions", comprehensions),
("string_literal", string_literal),
];
let (handles, state) = mk_multi_file_state_assert_no_errors(&files, Require::Everything);
let transaction = state.transaction();
Expand Down
21 changes: 16 additions & 5 deletions pyrefly/lib/report/glean/convert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,11 +1002,22 @@ impl GleanState<'_> {
let completions = |ty| solver.completions(ty, Some(attr_name), false, false);
// The class that defines `attr_name`, as found by the solver's own member
// lookup, so that an inherited member is named after its defining class.
let defining_type = |ty: Type| match &ty {
Type::ClassType(cls) | Type::SelfType(cls) => solver
.get_class_member_with_defining_class(cls.class_object(), attr_name)
.map_or(ty.clone(), |member| Type::ClassDef(member.defining_class)),
_ => ty,
let defining_type = |ty: Type| {
// A literal is an instance of its general class (e.g. `"a"` is a `str`),
// so its members are named after that class.
let ty = match ty {

@rchen152 rchen152 Nov 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Splitting the logic between two matchs makes it sort of hard to read. I'd recommend something like:

let cls = match &ty {
    ...  // extract the `ClassType`
    _ => return ty,
};
// get the class member, falling back to `Type::ClassType(cls.clone())`

@Quaden2307 Quaden2307 Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks for the feedback!

Type::Literal(lit) => {
Type::ClassType(lit.value.general_class_type(solver.stdlib).clone())
}
Type::LiteralString(_) => Type::ClassType(solver.stdlib.str().clone()),
ty => ty,
};
match &ty {
Type::ClassType(cls) | Type::SelfType(cls) => solver
.get_class_member_with_defining_class(cls.class_object(), attr_name)
.map_or(ty.clone(), |member| Type::ClassDef(member.defining_class)),
_ => ty,
}
};

let tys = match base_type.clone() {
Expand Down
Loading
Loading