Skip to content

Conversation

@AlessandroMinoccheri
Copy link
Member

This PR should fix #275

@codecov-commenter
Copy link

Codecov Report

Merging #276 (1422ac5) into main (5d77762) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #276      +/-   ##
============================================
+ Coverage     93.51%   93.55%   +0.03%     
- Complexity      363      367       +4     
============================================
  Files            57       57              
  Lines           971      977       +6     
============================================
+ Hits            908      914       +6     
  Misses           63       63              
Impacted Files Coverage Δ
src/CLI/Runner.php 95.83% <ø> (ø)
src/Rules/ArchRule.php 100.00% <ø> (ø)
src/Rules/Constraints.php 100.00% <ø> (ø)
src/Analyzer/FileVisitor.php 82.14% <100.00%> (+1.37%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

$violations = new Violations();

$notHaveDependencyOutsideNamespace = new Implement('MyInterface');
$notHaveDependencyOutsideNamespace->evaluate($cd[0], $violations, 'we want to add this rule for our software');
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure I got the goal of this test

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a simple rule @micheleorselli to check if we are parsing or not the enum class.
Without the code inside the FileVisitor the enum class is not parsed and this test it's red.
We can try to find a different test to be more explicit. Any ideas?

$this->classDescriptions[] = $classDescription;
}

if ($node instanceof Node\Stmt\Enum_ && null !== $this->classDescriptionBuilder) {
Copy link
Member

Choose a reason for hiding this comment

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

Here we treat enum as a class which is fine for now. I am wondering if we should add a method like isEnum or similar

Copy link
Member Author

Choose a reason for hiding this comment

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

What is the benefit to add the method isEnum now?
I think it could be fine.

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.

Parse Enum

4 participants