Skip to content

Commit d78ee47

Browse files
committed
[ty] Don't suggest things that aren't subclasses of BaseException after raise
This only applies to items that have a type associated with them. That is, things that are already in scope. For items that don't have a type associated with them (i.e., suggestions from auto-import), we still suggest them since we can't know if they're appropriate or not. It's not quite clear on how best to improve here for the auto-import case. (Short of, say, asking for the type of each such symbol. But the performance implications of that aren't known yet.) Note that because of auto-import, we were still suggesting `NotImplemented` even though astral-sh/ty#1262 specifically cites it as the motivating example that we *shouldn't* suggest. This was occuring because auto-import was including symbols from the `builtins` module, even though those are actually already in scope. So this PR also gets rid of those suggestions from auto-import. Overall, this means that, at least, `raise NotImpl` won't suggest `NotImplemented`. Fixes astral-sh/ty#1262
1 parent 03dfbf2 commit d78ee47

File tree

7 files changed

+52
-13
lines changed

7 files changed

+52
-13
lines changed

crates/ty_completion_eval/completion-evaluation-tasks.csv

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ numpy-array,main.py,1,1
1818
object-attr-instance-methods,main.py,0,1
1919
object-attr-instance-methods,main.py,1,1
2020
pass-keyword-completion,main.py,0,1
21-
raise-uses-base-exception,main.py,0,2
21+
raise-uses-base-exception,main.py,0,1
2222
scope-existing-over-new-import,main.py,0,1
2323
scope-prioritize-closer,main.py,0,2
2424
scope-simple-long-identifier,main.py,0,1
2525
tstring-completions,main.py,0,1
2626
ty-extensions-lower-stdlib,main.py,0,8
2727
type-var-typing-over-ast,main.py,0,3
28-
type-var-typing-over-ast,main.py,1,279
28+
type-var-typing-over-ast,main.py,1,278

crates/ty_ide/src/completion.rs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use ruff_python_codegen::Stylist;
1010
use ruff_python_parser::{Token, TokenAt, TokenKind, Tokens};
1111
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
1212
use ty_python_semantic::{
13-
Completion as SemanticCompletion, ModuleName, NameKind, SemanticModel,
14-
types::{CycleDetector, Type},
13+
Completion as SemanticCompletion, KnownModule, ModuleName, NameKind, SemanticModel,
14+
types::{CycleDetector, KnownClass, Type},
1515
};
1616

1717
use crate::docstring::Docstring;
@@ -82,6 +82,11 @@ impl<'db> Completions<'db> {
8282
fn force_add(&mut self, completion: Completion<'db>) {
8383
self.items.push(completion);
8484
}
85+
86+
/// Removes any completion that doesn't satisfy the given predicate.
87+
fn retain(&mut self, predicate: impl FnMut(&Completion<'_>) -> bool) {
88+
self.items.retain(predicate);
89+
}
8590
}
8691

8792
impl<'db> Extend<SemanticCompletion<'db>> for Completions<'db> {
@@ -364,6 +369,14 @@ pub fn completion<'db>(
364369
}
365370
}
366371

372+
if is_raising_exception(tokens) {
373+
let type_base_exception = KnownClass::BaseException.to_subclass_of(db);
374+
completions.retain(|c| {
375+
let Some(ty) = c.ty else { return true };
376+
ty.is_assignable_to(db, type_base_exception)
377+
});
378+
}
379+
367380
completions.into_completions()
368381
}
369382

@@ -427,7 +440,8 @@ fn add_unimported_completions<'db>(
427440
let members = importer.members_in_scope_at(scoped.node, scoped.node.start());
428441

429442
for symbol in all_symbols(db, &completions.query) {
430-
if symbol.module.file(db) == Some(file) {
443+
if symbol.module.file(db) == Some(file) || symbol.module.is_known(db, KnownModule::Builtins)
444+
{
431445
continue;
432446
}
433447

@@ -1358,6 +1372,30 @@ fn is_in_variable_binding(parsed: &ParsedModuleRef, offset: TextSize, typed: Opt
13581372
})
13591373
}
13601374

1375+
/// Returns true when the cursor is after a `raise` keyword.
1376+
fn is_raising_exception(tokens: &[Token]) -> bool {
1377+
/// The maximum number of tokens we're willing to
1378+
/// look-behind to find a `raise` keyword.
1379+
const LIMIT: usize = 10;
1380+
1381+
// This only looks for things like `raise foo.bar.baz.qu<CURSOR>`.
1382+
// Technically, any kind of expression is allowed after `raise`.
1383+
// But we may not always wanted to treat it specially. So we're
1384+
// rather conservative about what we consider "raising an
1385+
// exception" to be for the purposes of completions. The failure
1386+
// mode here is that we may wind up suggesting things that
1387+
// shouldn't be raised. The benefit is that when this heuristic
1388+
// does work, we won't suggest things that shouldn't be raised.
1389+
for token in tokens.iter().rev().take(LIMIT) {
1390+
match token.kind() {
1391+
TokenKind::Name | TokenKind::Dot => continue,
1392+
TokenKind::Raise => return true,
1393+
_ => return false,
1394+
}
1395+
}
1396+
false
1397+
}
1398+
13611399
/// Order completions according to the following rules:
13621400
///
13631401
/// 1) Names with no underscore prefix

crates/ty_python_semantic/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ pub use db::Db;
1212
pub use diagnostic::add_inferred_python_version_hint_to_diagnostic;
1313
pub use module_name::{ModuleName, ModuleNameResolutionError};
1414
pub use module_resolver::{
15-
Module, SearchPath, SearchPathValidationError, SearchPaths, all_modules, list_modules,
16-
resolve_module, resolve_real_module, system_module_search_paths,
15+
KnownModule, Module, SearchPath, SearchPathValidationError, SearchPaths, all_modules,
16+
list_modules, resolve_module, resolve_real_module, system_module_search_paths,
1717
};
1818
pub use program::{
1919
Program, ProgramSettings, PythonVersionFileSource, PythonVersionSource,

crates/ty_python_semantic/src/module_resolver/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::iter::FusedIterator;
22

33
pub use list::{all_modules, list_modules};
4-
pub(crate) use module::KnownModule;
4+
pub use module::KnownModule;
55
pub use module::Module;
66
pub use path::{SearchPath, SearchPathValidationError};
77
pub use resolver::SearchPaths;

crates/ty_python_semantic/src/module_resolver/module.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ impl<'db> Module<'db> {
6767
}
6868

6969
/// Does this module represent the given known module?
70-
pub(crate) fn is_known(self, db: &'db dyn Database, known_module: KnownModule) -> bool {
70+
pub fn is_known(self, db: &'db dyn Database, known_module: KnownModule) -> bool {
7171
self.known(db) == Some(known_module)
7272
}
7373

crates/ty_python_semantic/src/types.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ use crate::types::variance::VarianceInferable;
7474
use crate::types::visitor::{any_over_type, exceeds_max_specialization_depth};
7575
use crate::unpack::EvaluationMode;
7676
use crate::{Db, FxOrderSet, Module, Program};
77-
pub(crate) use class::{ClassLiteral, ClassType, GenericAlias, KnownClass};
77+
pub use class::KnownClass;
78+
pub(crate) use class::{ClassLiteral, ClassType, GenericAlias};
7879
use instance::Protocol;
7980
pub use instance::{NominalInstanceType, ProtocolInstanceType};
8081
pub use special_form::SpecialFormType;
@@ -1662,8 +1663,8 @@ impl<'db> Type<'db> {
16621663

16631664
/// Return true if this type is assignable to type `target`.
16641665
///
1665-
/// See [`TypeRelation::Assignability`] for more details.
1666-
pub(crate) fn is_assignable_to(self, db: &'db dyn Db, target: Type<'db>) -> bool {
1666+
/// See `TypeRelation::Assignability` for more details.
1667+
pub fn is_assignable_to(self, db: &'db dyn Db, target: Type<'db>) -> bool {
16671668
self.when_assignable_to(db, target, InferableTypeVars::None)
16681669
.is_always_satisfied(db)
16691670
}

crates/ty_python_semantic/src/types/class.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4823,7 +4823,7 @@ impl KnownClass {
48234823
/// representing that class and all possible subclasses of the class.
48244824
///
48254825
/// If the class cannot be found in typeshed, a debug-level log message will be emitted stating this.
4826-
pub(crate) fn to_subclass_of(self, db: &dyn Db) -> Type<'_> {
4826+
pub fn to_subclass_of(self, db: &dyn Db) -> Type<'_> {
48274827
self.to_class_literal(db)
48284828
.to_class_type(db)
48294829
.map(|class| SubclassOfType::from(db, class))

0 commit comments

Comments
 (0)