Skip to content
Merged
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
4 changes: 4 additions & 0 deletions _typos.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ extend-exclude = [
"crates/ty_vendored/vendor/**/*",
"**/resources/**/*",
"**/snapshots/**/*",
# Completion tests tend to have a lot of incomplete
# words naturally. It's annoying to have to make all
# of them actually words. So just ignore typos here.
"crates/ty_ide/src/completion.rs",
]

[default.extend-words]
Expand Down
156 changes: 156 additions & 0 deletions crates/ty_ide/src/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,162 @@ print(f\"{some<CURSOR>
");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_function_identifier1() {
let test = cursor_test(
"\
def m<CURSOR>
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_function_identifier2() {
let test = cursor_test(
"\
def m<CURSOR>(): pass
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn fscope_id_missing_function_identifier3() {
let test = cursor_test(
"\
def m(): pass
<CURSOR>
",
);

assert_snapshot!(test.completions(), @r"
m
");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_class_identifier1() {
let test = cursor_test(
"\
class M<CURSOR>
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_type_alias1() {
let test = cursor_test(
"\
Fo<CURSOR> = float
",
);

assert_snapshot!(test.completions(), @r"
Fo
float
");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_import1() {
let test = cursor_test(
"\
import fo<CURSOR>
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_import2() {
let test = cursor_test(
"\
import foo as ba<CURSOR>
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_from_import1() {
let test = cursor_test(
"\
from fo<CURSOR> import wat
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_from_import2() {
let test = cursor_test(
"\
from foo import wa<CURSOR>
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_from_import3() {
let test = cursor_test(
"\
from foo import wat as ba<CURSOR>
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_try_except1() {
let test = cursor_test(
"\
try:
pass
except Type<CURSOR>:
pass
",
);

assert_snapshot!(test.completions(), @r"
Type
");
}

// Ref: https://github.com/astral-sh/ty/issues/572
#[test]
fn scope_id_missing_global1() {
let test = cursor_test(
"\
def _():
global fo<CURSOR>
",
);

assert_snapshot!(test.completions(), @"<No completions found>");
}

impl CursorTest {
fn completions(&self) -> String {
let completions = completion(&self.db, self.file, self.cursor_offset);
Expand Down
8 changes: 8 additions & 0 deletions crates/ty_python_semantic/src/semantic_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,14 @@ impl<'db> SemanticIndex<'db> {
self.scopes_by_expression[&expression.into()]
}

/// Returns the ID of the `expression`'s enclosing scope.
pub(crate) fn try_expression_scope_id(
&self,
expression: impl Into<ExpressionNodeKey>,
) -> Option<FileScopeId> {
self.scopes_by_expression.get(&expression.into()).copied()
}

/// Returns the [`Scope`] of the `expression`'s enclosing scope.
#[allow(unused)]
#[track_caller]
Expand Down
15 changes: 11 additions & 4 deletions crates/ty_python_semantic/src/semantic_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,22 @@ impl<'db> SemanticModel<'db> {
/// scope of this model's `File` are returned.
pub fn completions(&self, node: ast::AnyNodeRef<'_>) -> Vec<Name> {
let index = semantic_index(self.db, self.file);
let file_scope = match node {
ast::AnyNodeRef::Identifier(identifier) => index.expression_scope_id(identifier),

// TODO: We currently use `try_expression_scope_id` here as a hotfix for [1].
// Revert this to use `expression_scope_id` once a proper fix is in place.
Copy link
Contributor

Choose a reason for hiding this comment

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

(Not a request to actually change this comment, but) I'm not convinced we should necessarily ever change this back to using expression_scope_id, regardless of what other fixes are made.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Admittedly, I didn't follow the full discussion in #18441, so I was potentially overly-cautious here. I opened this PR mainly because I can't work without the ty playground anymore 🙃.

//
// [1] https://github.com/astral-sh/ty/issues/572
let Some(file_scope) = (match node {
ast::AnyNodeRef::Identifier(identifier) => index.try_expression_scope_id(identifier),
node => match node.as_expr_ref() {
// If we couldn't identify a specific
// expression that we're in, then just
// fall back to the global scope.
None => FileScopeId::global(),
Some(expr) => index.expression_scope_id(expr),
None => Some(FileScopeId::global()),
Some(expr) => index.try_expression_scope_id(expr),
},
}) else {
return vec![];
};
let mut symbols = vec![];
for (file_scope, _) in index.ancestor_scopes(file_scope) {
Expand Down
Loading