Skip to content

Conversation

asukaminato0721
Copy link
Contributor

fix #1200

@meta-cla meta-cla bot added the cla signed label Oct 16, 2025
@asukaminato0721 asukaminato0721 marked this pull request as ready for review October 16, 2025 18:52
@asukaminato0721 asukaminato0721 changed the title fix 1200 fix error on definition for implicit abstract classes Oct 16, 2025
Copy link
Contributor

@yangdanny97 yangdanny97 left a comment

Choose a reason for hiding this comment

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

Back to you with some comments

@asukaminato0721 asukaminato0721 marked this pull request as draft October 17, 2025 06:20
@asukaminato0721
Copy link
Contributor Author

asukaminato0721 commented Oct 17, 2025

Seems that 6f6634d will fail...

extends_abc() returns true even when the class inherits an ABC only indirectly, because the flag is propagated through base metadata.

Child(Base) now satisfies extends_abc() == true, so the solver skips emitting ErrorKind::ImplicitAbstractClass.

failures:

---- test::abstract_methods::test_abstract_async_iterator stdout ----
DEBUG Running with 3 threads (5 MiB stack size)
DEBUG Running epoch 1 of run 0
DEBUG Committing transaction
ERROR Abstract methods for async generators should use `def`, not `async def` [bad-function-definition]
   --> pyrefly/lib/test/abstract_methods.rs:240:15
    |
240 |     async def foo(self) -> AsyncIterator[int]:  # E: Abstract methods for async generators should use `def`, not `async def`
    |               ^^^
    |
Error: Expectations failed for in-memory pyrefly/lib/test/abstract_methods.rs: expected 2 errors, but got 1

---- test::abstract_methods::test_abstract_property stdout ----
DEBUG Running with 3 threads (5 MiB stack size)
DEBUG Running epoch 1 of run 0
DEBUG Committing transaction
ERROR Cannot instantiate `Child` because the following members are abstract: `processor` [bad-instantiation]
   --> pyrefly/lib/test/abstract_methods.rs:225:10
    |
225 | x = Child()  # E: Cannot instantiate `Child`
    |          ^^
    |
Error: Expectations failed for in-memory pyrefly/lib/test/abstract_methods.rs: expected 2 errors, but got 1

---- test::abstract_methods::test_inherited_abstract_method stdout ----
DEBUG Running with 3 threads (5 MiB stack size)
DEBUG Running epoch 1 of run 0
DEBUG Committing transaction
ERROR Cannot instantiate `Child` because the following members are abstract: `method` [bad-instantiation]
   --> pyrefly/lib/test/abstract_methods.rs:102:14
    |
102 | child = Child()  # E: Cannot instantiate `Child`
    |              ^^
    |
Error: Expectations failed for in-memory pyrefly/lib/test/abstract_methods.rs: expected 2 errors, but got 1

---- test::abstract_methods::test_partial_implementation stdout ----
DEBUG Running with 3 threads (5 MiB stack size)
DEBUG Running epoch 1 of run 0
DEBUG Committing transaction
ERROR Cannot instantiate `Partial` because the following members are abstract: `method2` [bad-instantiation]
   --> pyrefly/lib/test/abstract_methods.rs:144:12
    |
144 | p = Partial()  # E: Cannot instantiate `Partial`
    |            ^^
    |
Error: Expectations failed for in-memory pyrefly/lib/test/abstract_methods.rs: expected 2 errors, but got 1


failures:
    test::abstract_methods::test_abstract_async_iterator
    test::abstract_methods::test_abstract_property
    test::abstract_methods::test_inherited_abstract_method
    test::abstract_methods::test_partial_implementation

@yangdanny97
Copy link
Contributor

Let me import and investigate

Copy link

meta-codesync bot commented Oct 20, 2025

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D85072634.

@yangdanny97
Copy link
Contributor

You're right, extends_abc will return true if it's transitive, we'd need a similar check for a class directly extending ABC. I'll see if I can patch this

@yangdanny97 yangdanny97 marked this pull request as ready for review October 20, 2025 21:28
@yangdanny97
Copy link
Contributor

I believe I've managed to patch it internally. Instead of storing the metaclass as Option, I made an enum that encodes its origin.

Copy link
Contributor

@stroxler stroxler left a comment

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.

@meta-codesync meta-codesync bot closed this in 5e9b54b Oct 21, 2025
Copy link

meta-codesync bot commented Oct 21, 2025

@yangdanny97 merged this pull request in 5e9b54b.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

error on definition for implicit abstract classes

4 participants