Skip to content

Refactor all the things#2

Merged
daniel-cannon merged 3 commits intoalphafrom
fix-fqcn-from-path
Mar 23, 2026
Merged

Refactor all the things#2
daniel-cannon merged 3 commits intoalphafrom
fix-fqcn-from-path

Conversation

@daniel-cannon
Copy link
Copy Markdown
Member

@daniel-cannon daniel-cannon commented Mar 23, 2026

🤷

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors how interface-union types are generated by introducing a token-based PHP parser for extracting class metadata (namespace/class/extends/implements/imports), adds fixtures and tests around edge cases (aliases, grouped uses, inheritance, long declarations), and updates development/CI tooling.

Changes:

  • Add a parser in BaseCommand to extract class info from PHP source and update interface-union generation to use it (including inherited-interface detection).
  • Expand PHPUnit coverage with workbench fixtures and parser-focused tests for imports/aliases/grouped uses/extends and chunk-boundary reading.
  • Update Docker image dependencies and narrow the GitHub Actions Laravel matrix.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
workbench/app/Models/SomeLongClassNameThatCrossesTheLegacyChunkBoundary.php Fixture for chunk-boundary declaration parsing
workbench/app/Models/ParentAnimal.php Fixture for inheritance chain
workbench/app/Models/InheritedAnimal.php Fixture for extends parsing
workbench/app/Models/GroupedUseAnimal.php Fixture for grouped use + multiple interfaces
workbench/app/Models/FullyQualifiedAnimal.php Fixture for fully-qualified implements
workbench/app/Models/AliasAnimal.php Fixture for aliased imports
workbench/app/Interfaces/CompanionInterface.php Additional interface fixture for grouped import test
tests/TestableGenerateInterfaceUnionsCommand.php Test helper exposing parser method
tests/TestCase.php Import formatting cleanup
tests/Commands/GenerateInterfaceUnionsCommandTest.php New/updated tests for command output and parser behavior
src/Commands/GenerateInterfaceUnionsCommand.php Switch to parsed class info + inheritance traversal instead of is_subclass_of
src/Commands/GenerateFormRequestsCommand.php Formatting/brace cleanup (no functional change apparent)
src/Commands/BaseCommand.php Add class-declaration reader + token parser for namespace/imports/extends/implements
Dockerfile Add SimpleXML build deps and default Laravel version
.github/workflows/php-test.yaml Reduce Laravel test matrix versions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…te dependencies, and add new tests for edge cases.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…extend functionality, update parsing logic in `BaseCommand`, and add tests for new attribute handling.
@daniel-cannon daniel-cannon marked this pull request as ready for review March 23, 2026 15:28
Copy link
Copy Markdown
Member

@edenchazard edenchazard left a comment

Choose a reason for hiding this comment

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

If you're positive it works then sure 👍 Looks like it tries to compensate for edge cases where the boundary of namespace, class or implements, crosses two batches of tokens.

Comment on lines +7 to +9
/**
* Padding: 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I love it lol

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I robotted most of it. 😬

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I could tell lol

But this does make sense, you're adding excess to artificially force the edge case you ran into.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll smash it in then. 🙏

@daniel-cannon daniel-cannon merged commit 0263204 into alpha Mar 23, 2026
6 checks passed
@daniel-cannon daniel-cannon deleted the fix-fqcn-from-path branch March 23, 2026 16:22
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.

3 participants