Skip to content

fix: clarify implicit abstract class diagnostic#1824

Open
popsiclelmlm wants to merge 1 commit into
DetachHead:mainfrom
popsiclelmlm:fix/abstract-class-message
Open

fix: clarify implicit abstract class diagnostic#1824
popsiclelmlm wants to merge 1 commit into
DetachHead:mainfrom
popsiclelmlm:fix/abstract-class-message

Conversation

@popsiclelmlm

Copy link
Copy Markdown

Summary

Fixes #1747 by clarifying the reportImplicitAbstractClass diagnostic so it does not imply that the missing abstract symbol must come from the parent abstract class.

Reproduction

The issue example is a class that extends an abstract base and declares a new abstract method itself:

from abc import ABC, abstractmethod

class A(ABC): ...

class B(A):
    @abstractmethod
    def f(self): ...

The previous wording said the class extends an abstract class “without implementing all abstract symbols,” which can read as if the missing abstract symbol necessarily belongs to the base class.

Root cause

The diagnostic message was correct for some cases, but too narrow for classes that still contain abstract symbols introduced on the subclass itself.

Changes

  • Update the English classImplicitlyAbstract diagnostic wording to say the class still contains abstract symbols.
  • Add a sample case where the subclass introduces an abstract method.
  • Assert the updated diagnostic message for both an inherited abstract symbol and a subclass-defined abstract symbol.

Tests

  • git diff --check
  • cd packages/pyright-internal && node --max-old-space-size=8192 --expose-gc ./node_modules/jest/bin/jest checker.test.ts -t reportImplicitAbstractClass --runInBand --forceExit
  • npm run typecheck
  • npx prettier -c packages/pyright-internal/src/localization/package.nls.en-us.json packages/pyright-internal/src/tests/checker.test.ts
  • ESLINT_USE_FLAT_CONFIG=false npx eslint packages/pyright-internal/src/tests/checker.test.ts

Screenshots/Logs

N/A

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bit confusing error message about abstract class

1 participant