Detect the use of Attributes for PHPUnit Sniffs #207
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #207 +/- ##
============================================
+ Coverage 98.28% 98.34% +0.06%
- Complexity 959 1000 +41
============================================
Files 40 42 +2
Lines 2971 3140 +169
============================================
+ Hits 2920 3088 +168
- Misses 51 52 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
andrewnicols
force-pushed
the
testcaseAttributes
branch
2 times, most recently
from
ea1cd85 to
b45d6fc
Compare
andrewnicols
force-pushed
the
testcaseAttributes
branch
from
b45d6fc to
064349d
Compare
andrewnicols
force-pushed
the
testcaseAttributes
branch
from
064349d to
52c002b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements support for detecting PHP 8.0+ attributes in PHPUnit test case sniffs, specifically adding support for #[DataProvider] and other PHPUnit attributes alongside existing phpdoc-based annotations.
- Adds a new
NamespaceScopeUtilhelper class to properly qualify class names considering imports and namespaces - Extends PHPUnit sniffs to support both traditional phpdoc comments and modern PHP attributes
- Creates a shared
AbstractTestCaseSniffbase class to consolidate common functionality
Reviewed Changes
Copilot reviewed 14 out of 39 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| moodle/Util/NamespaceScopeUtil.php | New utility class for qualifying class names within namespace scope |
| moodle/Util/Attributes.php | Enhanced with qualified name support and new helper methods |
| moodle/Tests/Util/NamespaceScopeUtilTest.php | Test coverage for the new namespace utility |
| moodle/Tests/Util/AttributesTest.php | Updated tests for enhanced attribute functionality |
| moodle/Tests/Sniffs/PHPUnit/fixtures/Provider/attributes_test.php | Test fixture for attribute-based data providers |
| moodle/Tests/Sniffs/PHPUnit/fixtures/Covers/testcasecovers_method_attribute.php | Test fixture for method-level coverage attributes |
| moodle/Tests/Sniffs/PHPUnit/fixtures/Covers/testcasecovers_attribute.php | Test fixture for class-level coverage attributes |
| moodle/Tests/Sniffs/PHPUnit/fixtures/Covers/testcasecovers_abstract.php | Test fixture for abstract test classes |
| moodle/Tests/Sniffs/PHPUnit/TestCaseProviderTest.php | Updated test with attribute support cases |
| moodle/Tests/Sniffs/PHPUnit/TestCaseCoversTest.php | Updated test with attribute support cases |
| moodle/Sniffs/PHPUnit/TestCaseProviderSniff.php | Enhanced to detect and validate DataProvider attributes |
| moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php | Enhanced to detect and validate coverage attributes |
| moodle/Sniffs/PHPUnit/AbstractTestCaseSniff.php | New base class for PHPUnit sniffs with shared functionality |
| CHANGELOG.md | Documentation of the changes |
| use Example\Thing\AnotherExample; | ||
| ', | ||
| T_CLASS, | ||
| ['Example' => \Example\Thing\Example::class], |
There was a problem hiding this comment.
The test is using \Example\Thing\Example::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: ['Example' => 'Example\Thing\Example'].
| ['Example' => \Example\Thing\Example::class], | |
| ['Example' => 'Example\Thing\Example'], |
Copilot uses AI. Check for mistakes.
| use Example\Thing\AnotherExample; | ||
| ', | ||
| T_CLASS, | ||
| ['OtherExample' => \Example\Thing\Example::class], |
There was a problem hiding this comment.
The test is using \Example\Thing\Example::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: ['OtherExample' => 'Example\Thing\Example'].
| ['OtherExample' => \Example\Thing\Example::class], | |
| ['OtherExample' => 'Example\Thing\Example'], |
Copilot uses AI. Check for mistakes.
| 'Another' => \My\Full\Classname::class, | ||
| 'NSname' => \My\Full\NSname::class, | ||
| 'ClassA' => \some\name\space\ClassA::class, | ||
| 'ClassB' => \some\name\space\ClassB::class, | ||
| 'C' => \some\name\space\ClassC::class, |
There was a problem hiding this comment.
The test is using \My\Full\Classname::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: 'Another' => 'My\Full\Classname'.
| 'Another' => \My\Full\Classname::class, | |
| 'NSname' => \My\Full\NSname::class, | |
| 'ClassA' => \some\name\space\ClassA::class, | |
| 'ClassB' => \some\name\space\ClassB::class, | |
| 'C' => \some\name\space\ClassC::class, | |
| 'Another' => 'My\Full\Classname', | |
| 'NSname' => 'My\Full\NSname', | |
| 'ClassA' => 'some\name\space\ClassA', | |
| 'ClassB' => 'some\name\space\ClassB', | |
| 'C' => 'some\name\space\ClassC', |
Copilot uses AI. Check for mistakes.
| 'Another' => \My\Full\Classname::class, | ||
| 'NSname' => \My\Full\NSname::class, | ||
| 'ClassA' => \some\name\space\ClassA::class, | ||
| 'ClassB' => \some\name\space\ClassB::class, | ||
| 'C' => \some\name\space\ClassC::class, |
There was a problem hiding this comment.
The test is using \My\Full\NSname::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: 'NSname' => 'My\Full\NSname'.
| 'Another' => \My\Full\Classname::class, | |
| 'NSname' => \My\Full\NSname::class, | |
| 'ClassA' => \some\name\space\ClassA::class, | |
| 'ClassB' => \some\name\space\ClassB::class, | |
| 'C' => \some\name\space\ClassC::class, | |
| 'Another' => 'My\Full\Classname', | |
| 'NSname' => 'My\Full\NSname', | |
| 'ClassA' => 'some\name\space\ClassA', | |
| 'ClassB' => 'some\name\space\ClassB', | |
| 'C' => 'some\name\space\ClassC', |
Copilot uses AI. Check for mistakes.
| 'ClassA' => \some\name\space\ClassA::class, | ||
| 'ClassB' => \some\name\space\ClassB::class, | ||
| 'C' => \some\name\space\ClassC::class, |
There was a problem hiding this comment.
The test is using \some\name\space\ClassA::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: 'ClassA' => 'some\name\space\ClassA'.
| 'ClassA' => \some\name\space\ClassA::class, | |
| 'ClassB' => \some\name\space\ClassB::class, | |
| 'C' => \some\name\space\ClassC::class, | |
| 'ClassA' => 'some\name\space\ClassA', | |
| 'ClassB' => 'some\name\space\ClassB', | |
| 'C' => 'some\name\space\ClassC', |
Copilot uses AI. Check for mistakes.
| 'NSname' => \My\Full\NSname::class, | ||
| 'ClassA' => \some\name\space\ClassA::class, | ||
| 'ClassB' => \some\name\space\ClassB::class, | ||
| 'C' => \some\name\space\ClassC::class, |
There was a problem hiding this comment.
The test is using \some\name\space\ClassC::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: 'C' => 'some\name\space\ClassC'.
| 'C' => \some\name\space\ClassC::class, | |
| 'C' => 'some\name\space\ClassC', |
Copilot uses AI. Check for mistakes.
| class Example { | ||
| }', | ||
| T_CLASS, | ||
| \Example\Attribute::class, |
There was a problem hiding this comment.
The test is using \Example\Attribute::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: 'Example\Attribute'.
| \Example\Attribute::class, | |
| 'Example\Attribute', |
Copilot uses AI. Check for mistakes.
| class Example { | ||
| }', | ||
| T_CLASS, | ||
| \Example\MyFirstAttribute::class, |
There was a problem hiding this comment.
The test is using \Example\MyFirstAttribute::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: 'Example\MyFirstAttribute'.
| \Example\MyFirstAttribute::class, | |
| 'Example\MyFirstAttribute', |
Copilot uses AI. Check for mistakes.
| class Example { | ||
| }', | ||
| T_CLASS, | ||
| \Example\MyFirstAttribute::class, |
There was a problem hiding this comment.
The test is using \Example\MyFirstAttribute::class which assumes the class exists, but this will cause a fatal error during test execution. Use a string representation instead: 'Example\MyFirstAttribute'.
| \Example\MyFirstAttribute::class, | |
| 'Example\MyFirstAttribute', |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| // Ignore any classname which does not end in "_test" or "_testcase". | ||
| if (substr($className, -5) !== '_test' && substr($className, -10) !== '_testcase') { |
There was a problem hiding this comment.
The condition checking for '_testcase' suffix is incorrect. It checks for -10 characters but '_testcase' is 9 characters long. Should be substr($className, -9) !== '_testcase'.
| if (substr($className, -5) !== '_test' && substr($className, -10) !== '_testcase') { | |
| if (substr($className, -5) !== '_test' && substr($className, -9) !== '_testcase') { |
Copilot uses AI. Check for mistakes.
andrewnicols
force-pushed
the
testcaseAttributes
branch
from
52c002b to
68254b8
Compare
Fixes #205
Blocked by #206 and #207