Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 15 additions & 4 deletions pyrefly/lib/lsp/wasm/type_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,11 @@ mod impl_ {
let Some(identifier_with_context) = transaction.identifier_at(handle, position) else {
return Vec::new();
};
let key = match identifier_with_context.context {
let is_attribute_hover = matches!(
&identifier_with_context.context,
IdentifierContext::Attribute { .. }
);
let key = match &identifier_with_context.context {
IdentifierContext::Expr(expr_context) => match expr_context {
ExprContext::Store => {
Key::Definition(ShortIdentifier::new(&identifier_with_context.identifier))
Expand All @@ -216,9 +220,13 @@ mod impl_ {
Key::BoundName(ShortIdentifier::new(&identifier_with_context.identifier))
}
},
// Type sources are only meaningful for expression-context identifiers (variables,
// parameters). Other contexts like imports, type annotations, and decorators don't
// have narrowing or first-use inference semantics.
// Attribute hover should surface narrowing attached to the containing facet expression
// (`obj.field`) using the base variable's current flow binding.
IdentifierContext::Attribute {
base_identifier: Some(base_identifier),
expr_context: ExprContext::Load | ExprContext::Invalid,
..
} => Key::BoundName(ShortIdentifier::new(base_identifier)),
_ => return Vec::new(),
};
if !bindings.is_valid_key(&key) {
Expand All @@ -229,6 +237,9 @@ mod impl_ {
if let Some(narrow_source) = narrow_source_for_key(&bindings, &module, idx) {
sources.push(narrow_source);
}
if is_attribute_hover {
return sources;
}
if let Some(first_use_source) = first_use_source_for_key(&bindings, &module, &key, position)
{
sources.push(first_use_source);
Expand Down
11 changes: 11 additions & 0 deletions pyrefly/lib/state/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ pub(crate) enum IdentifierContext {
Attribute {
/// The range of just the base expression.
base_range: TextRange,
/// The root name of the base expression, when the base is an attribute chain rooted at a name.
base_identifier: Option<Identifier>,
/// The range of the entire expression.
range: TextRange,
/// Whether the attribute is being loaded, assigned to, or deleted.
Expand Down Expand Up @@ -592,11 +594,20 @@ impl IdentifierWithContext {
}

fn from_expr_attr(id: &Identifier, attr: &ExprAttribute) -> Self {
fn base_identifier(expr: &Expr) -> Option<Identifier> {
match expr {
Expr::Name(name) => Some(Ast::expr_name_identifier(name.clone())),
Expr::Attribute(attr) => base_identifier(attr.value.as_ref()),
_ => None,
}
}

let identifier = id.clone();
Self {
identifier,
context: IdentifierContext::Attribute {
base_range: attr.value.range(),
base_identifier: base_identifier(attr.value.as_ref()),
range: attr.range(),
expr_context: attr.ctx,
},
Expand Down
30 changes: 30 additions & 0 deletions pyrefly/lib/test/lsp/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,36 @@ def f(x: int | str | None) -> None:
);
}

#[test]
fn hover_type_source_attribute_narrow() {
let code = r#"
class C:
x: int | None

def f(c: C) -> None:
if c.x is not None:
c.x
# ^
"#;
let report = get_batched_lsp_operations_report(&[("main", code)], |state, handle, position| {
match get_hover(&state.transaction(), handle, position, false) {
Some(Hover {
contents: HoverContents::Markup(markup),
..
}) => markup.value,
_ => "None".to_owned(),
}
});
assert!(
report.contains("**Type source**"),
"Expected type source section in field hover, got: {report}"
);
assert!(
report.contains("c.x is not None"),
"Expected attribute narrow in field hover, got: {report}"
);
}

#[test]
fn hover_type_source_no_source_at_first_use_site() {
// When hovering at the first-use site itself, we should not show
Expand Down
Loading