Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Internal refactor to support more token types - part 1/??? #602

Merged
merged 16 commits into from
Jul 6, 2021
Merged

Internal refactor to support more token types - part 1/??? #602

merged 16 commits into from
Jul 6, 2021

Conversation

patrickkusebauch
Copy link
Collaborator

Relates: #582

@patrickkusebauch patrickkusebauch marked this pull request as draft June 26, 2021 17:27
dbrumann
dbrumann reviewed Jun 28, 2021
View reviewed changes
Copy link
Collaborator

@dbrumann dbrumann left a comment

Choose a reason for hiding this comment

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

General question, why is it TokenLikeName? To me ClassLikeName makes sense, because it can be something like a trait or interface, so it's not strictly speaking a class, but now we have a token and its name, or not?

src/AstRunner/AstMap/TokenLikeName.php Outdated Show resolved Hide resolved
src/Dependency/InheritanceFlatter.php Outdated Show resolved Hide resolved
src/Dependency/InheritanceFlatter.php Outdated Show resolved Hide resolved
src/LayerAnalyser.php Outdated Show resolved Hide resolved
@patrickkusebauch
Copy link
Collaborator Author

Naming things is hard, TokenName would probably make more sense?

@patrickkusebauch patrickkusebauch changed the title WIP: TokenLikeName refactor - part 1/??? Internal refactor to support more token types - part 1/??? Jul 1, 2021
@patrickkusebauch patrickkusebauch marked this pull request as ready for review July 1, 2021 23:51
@patrickkusebauch
Copy link
Collaborator Author

@dbrumann I think I am done with this pass. More changes might be needed, but I will discover those only when I will try to write another Visitor.

dbrumann
dbrumann reviewed Jul 6, 2021
View reviewed changes
src/AstRunner/AstMap/AstFileReference.php Show resolved Hide resolved
src/DependencyEmitter/UsesDependencyEmitter.php
{
public function getName(): string
{
return 'BasicDependencyEmitter';
return 'UsesDependencyEmitter';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a reminder for myself, docs/concepts.md contains a code snippet with the old emitter name that we probably should update later.

src/AstRunner/AstMap/AstTokenReference.php Show resolved Hide resolved
src/RulesetEngine/SkippedViolationHelper.php Show resolved Hide resolved
@dbrumann dbrumann merged commit d383a92 into qossmic:main Jul 6, 2021
@patrickkusebauch patrickkusebauch deleted the tokenLikeName branch July 6, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbrumann dbrumann dbrumann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@patrickkusebauch @dbrumann