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

Incomplete attempt at @todo capitalization enforcement. #174

Draft
wants to merge 1 commit into
base: 8.3.x
Choose a base branch
from
Draft

Incomplete attempt at @todo capitalization enforcement. #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coder_sniffer/Drupal/Sniffs/Commenting/TodoCommentSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private function checkTodoFormat(File $phpcsFile, int $stackPtr, string $comment
(?-i) # Reset to case-sensitive
(?! # Start another non-consuming look-ahead, this time negative
@todo\s # It has to match lower-case @todo followed by one space
(?!-|:)\S # and then any non-space except "-" or ":".
(?!-|:)[A-Z] # and then any capital letter except "-" or ":".
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is tricky. We want to warn about lower case characters, but allow everything else. Even lower case characters should be allowed in some cases, for example @todo some_function() is not a good name, refactor it to a better name.

I think we have code somewhere else in Coder that checks for such things in some comment standard, we should check that.

)/m';

if ((bool) preg_match($expression, $comment, $matches) === true) {
Expand Down
8 changes: 5 additions & 3 deletions tests/Drupal/Commenting/TodoCommentUnitTest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
* These are valid examples.
*
* @todo Valid.
* @todo valid with lower-case first letter
* @todo $can start with a $
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be valid, one could document a variable name like "@todo $entity could be NULL here, fix this later in foo_fcuntion()."

* @todo \also with backslash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, backslash should be valid. "@todo \Drupal should not be used here, implement dependency injection later".

* @todo Valid
* @todo Multiple words are valid.
* @todo Multiple words are valid
*
* These are all incorrect but can be fixed automatically.
*
Expand Down Expand Up @@ -38,6 +38,7 @@
* @todo
* @to-do
* @TODO
* @todo this should be uppercase
*/

/**
Expand All @@ -46,6 +47,7 @@
function foo() {
// These are valid examples.
// @todo Valid.
// These are invalid since they don't start with a capital letter.
// @todo valid with lower-case first letter
// @todo $can start with a $
// @todo \also with backslash
Expand Down
14 changes: 8 additions & 6 deletions tests/Drupal/Commenting/TodoCommentUnitTest.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
* These are valid examples.
*
* @todo Valid.
* @todo valid with lower-case first letter
* @todo $can start with a $
* @todo \also with backslash
* @todo Valid
* @todo Multiple words are valid.
* @todo Multiple words are valid
*
* These are all incorrect but can be fixed automatically.
*
Expand Down Expand Up @@ -38,6 +38,7 @@
* @todo
* @todo
* @todo
* @todo this should be uppercase
*/

/**
Expand All @@ -46,9 +47,10 @@
function foo() {
// These are valid examples.
// @todo Valid.
// @todo valid with lower-case first letter
// @todo $can start with a $
// @todo \also with backslash
// These are invalid since they don't start with a capital letter.
// @todo Error
// @todo Error
// @todo Error
// This is not a todo tag. It is a general comment and we do not want
// to do the standards checking here.
// These are all incorrect but can be fixed automatically.
Expand Down
4 changes: 3 additions & 1 deletion tests/Drupal/Commenting/TodoCommentUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ protected function getErrorList(string $testFile): array
*/
protected function getWarningList(string $testFile): array
{
$warningList = (array_fill_keys(range(16, 34), 1) + array_fill_keys(range(38, 40), 1) + array_fill_keys(range(55, 73), 1) + array_fill_keys(range(75, 77), 1));
$warningList = (
array_fill_keys(range(16, 34), 1) + array_fill_keys(range(38, 41), 1) + array_fill_keys(range(51, 53), 1) + array_fill_keys(range(57, 75), 1) + array_fill_keys(range(77, 79), 1)
);
return $warningList;

}//end getWarningList()
Expand Down
2 changes: 1 addition & 1 deletion tests/Drupal/good/good.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ function test3() {
// Comment one.
t('x');
// Comment two.
// @todo this is valid!
// @todo This is valid!
t('x');
// Goes on?
t('x');
Expand Down