Skip to content

impl Autocomplete Abstract, Protocol and method override #1589 - #5444

Open
asukaminato0721 wants to merge 10 commits into
meta:mainfrom
asukaminato0721:1589-1
Open

asukaminato0721 wants to merge 10 commits into
meta:mainfrom
asukaminato0721:1589-1

Conversation

@asukaminato0721

Copy link
Copy Markdown
Collaborator

Summary

Fixes part of #1589 #2207

microsoft/pylance-release#146

Implemented an “Implement abstract members” quick fix that derives missing abstract members via the solver, copies signatures/decorators/docstrings, and inserts stubs into the selected class, then wired it into LSP code actions.

  • I am a human, and I fully understand the code I am submitting.

Test Plan

add test

@meta-codesync

meta-codesync Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D128243979. (Because this pull request was imported automatically, there will not be any future comments.)

@github-actions

This comment has been minimized.

Generated annotations must use Python syntax and omit a bound receiver.
Keep diagnostic text out of generated Python annotations while preserving semantic references for imports.
Preserve parameter kinds, type parameters, and source-safe defaults when generating method signatures.
Retain overloads, async declarations, property setters, and class-variable annotations in generated members.
Keep class headers, docstrings, existing decorators, and indentation intact when inserting member implementations.
Collect inherited abstract members and generate local edits for the selected class. Cover method and property stubs.
Check protocol methods and class variables without turning data members into methods.
Use structured annotation references to preserve aliases and avoid collisions. Check imported callable contracts and base specialization.
Cover inline classes, missing final newlines, nested indentation, and existing decorators.
Register the action with quick fixes so LSP kind filters and automatic requests return the expected actions.
@github-actions

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@stroxler stroxler assigned stroxler and unassigned samwgoldman Nov 1, 2026

@stroxler stroxler left a comment

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.

Thanks! This is looking better, but it's a complicated feature and a fresh round of review caught quite a few new edge cases where the behavior doesn't seem quite right:

  1. Generic stubs insert unsupported syntax on older Python targets.
    A legacy TypeVar method becomes def identity[T](...), which fails to parse on Python 3.11. The renderer needs a target-version guard; declining unsupported generation is an acceptable minimal fix.
    [Location: signature renderer](

    if let Some(tparams) = tparams.filter(|p| !p.is_empty()) {
    self.text("[");
    for (i, param) in tparams.iter().enumerate() {
    if i > 0 {
    self.text(", ");
    }
    self.text(&param.display_name_with_prefix().to_string());
    match param.restriction() {
    Restriction::Bound(ty) => {
    self.text(": ");
    self.annotation(ty);
    }
    Restriction::Constraints(types) => {
    self.text(": (");
    for (i, ty) in types.iter().enumerate() {
    if i > 0 {
    self.text(", ");
    }
    self.annotation(ty);
    }
    self.text(")");
    }
    Restriction::Unrestricted => {}
    Restriction::ShapeExtension(_) => return None,
    }
    if let Some(default) = param.default() {
    self.text(" = ");
    self.annotation(default);
    }
    }
    self.text("]");
    )

  2. Returned generic callables lose their type-variable binder.
    def factory[T](self) -> Callable[[T], T] becomes def factory(self) -> Callable[[T], T]. Applying the action introduces two UnknownName errors for T. Preserve the quantification or decline annotations that cannot be represented safely.
    [Location: callable annotation rendering](

    if self.render_source_annotation
    && let Some((callable, _)) = t.toplevel_callable_signatures().next()
    {
    let bound_signature = if matches!(t, Type::BoundMethod(_)) {
    callable.strip_first_param()
    } else {
    None
    };
    let callable = bound_signature.as_ref().unwrap_or(callable);
    self.maybe_fmt_with_module("typing", "Callable", output)?;
    output.write_str("[")?;
    match &callable.params {
    Params::List(params)
    if params
    .items()
    .iter()
    .all(|p| matches!(p, Param::PosOnly(..) | Param::Pos(..))) =>
    {
    output.write_str("[")?;
    self.fmt_type_sequence(
    params.items().iter().map(|p| p.as_type()),
    ", ",
    false,
    output,
    )?;
    output.write_str("]")?;
    }
    Params::ParamSpec(prefix, param_spec) => {
    if prefix.is_empty() {
    self.fmt_helper_generic(param_spec, false, output)?;
    } else {
    self.maybe_fmt_with_module("typing", "Concatenate", output)?;
    output.write_str("[")?;
    for param in prefix {
    self.fmt_helper_generic(param.ty(), false, output)?;
    output.write_str(", ")?;
    }
    self.fmt_helper_generic(param_spec, false, output)?;
    output.write_str("]")?;
    }
    }
    _ => output.write_str("...")?,
    }
    output.write_str(", ")?;
    self.fmt_helper_generic(&callable.ret, false, output)?;
    return output.write_str("]");
    )

  3. Abstract async generators become coroutines.
    The generated async def contains only raise NotImplementedError, so it loses generator-ness. Pyrefly reports an incompatible override; executing async for produces a TypeError instead of reaching the stub. Preserve the function kind or generate a compatible synchronous stub.
    [Location: declaration and body generation](

    self.text(indent);
    self.text(if is_async { "async def " } else { "def " });
    self.text(member.name.as_str());
    self.signature(callable, tparams.map(|p| p.as_ref()), !is_static)?;
    self.text(":\n");
    if !overloaded {
    self.body(member, indent, indent_unit);
    } else {
    self.text(&format!("{indent}{indent_unit}..."));
    )

  4. Async properties acquire a second coroutine wrapper.
    An abstract async def value(self) -> int property becomes async def value(self) -> Coroutine[Any, Any, int], producing a BadOverride. Normalization changes the top-level signature, but rendering uses the separate getter stored in property metadata. Normalize the signature actually rendered.
    Normalization · [Getter selection](

    let property = member.ty.property_metadata();
    let getter = property.map(|p| &p.getter);
    let setter = property.and_then(|p| p.setter.as_ref());
    let is_cached = metadata.flags.is_cached_property;
    let is_static = metadata.flags.is_staticmethod;
    let is_class = metadata.flags.is_classmethod;
    let is_async = metadata.flags.is_async;
    let overloaded = member.ty.toplevel_callable_signatures().count() > 1;
    for (index, (callable, tparams)) in getter
    .unwrap_or(&member.ty)
    .toplevel_callable_signatures()
    .enumerate()
    )

  5. The module-import fallback can resolve annotations through a rebound name.
    With a local Payload collision and base = 42, the action inserts import base at the top but generates base.Payload after the rebinding. This introduces missing-attribute errors and breaks annotation evaluation. Use a collision-free module alias.
    [Location: import planning](

    let tracker = ImportTracker::from_ast(
    &ast,
    handle.module(),
    handle.path().is_init(),
    handle.sys_info().dupe(),
    );
    let direct = tracker.direct_import_candidates(&parts, handle.module());
    let direct_refs = direct
    .iter()
    .map(|(module, name)| (*module, name.as_str()))
    .collect::<Vec<_>>();
    let (combined, missing) = tracker.resolve_annotation(&parts, handle.module(), &direct_refs);
    let mut imports = String::new();
    for (module, name) in direct {
    let import_handle = transaction.import_handle(handle, module, None).finding()?;
    let edit = insert_import_edit(
    &ast,
    transaction.config_finder(),
    handle.dupe(),
    import_handle,
    &name,
    ImportFormat::Absolute,
    );
    imports.push_str(&edit.insert_text);
    }
    let mut missing = missing.into_iter().collect::<Vec<_>>();
    missing.sort_by(|a, b| a.as_str().cmp(b.as_str()));
    for module in missing {
    let import_handle = transaction.import_handle(handle, module, None).finding()?;
    let (_, text, _) = import_regular_import_edit(&ast, import_handle, None);
    imports.push_str(&text);
    )

@stroxler stroxler left a comment

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.

Review automatically exported from Phabricator review in Meta.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants