The "missing documentation" ones must be removed and fixed. We cannot leave the code undocumented and hide it this way.

The ClassMatchesFilename ones are tough; there are several places were more than one class is defined in the same file. You could argue that we should separate them into their own files, but that breaks the history of the code (e.g. with git blame). But we can at least fix the WrongCase issues and get rid of that rule (that would be a simple file rename, which won't interfere with walking through the git history).

Oh and the NewLineComment rule is also something that must be fixed and removed.

Here are some stats on how many errors need to be fixed if each rule is to be turned off:...

[UPDATE: Table was moved to task description]

If I were to start somewhere, I would start with getting rid of $wgTitle. It appears in one function in AbuseFilter.class.php and seems to have been added in an effort to fix T57435 but no one has revisited that piece of code in two years.

Change 385410 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/extensions/AbuseFilter@master] Add missing documentation for protected functions

https://gerrit.wikimedia.org/r/385410

Change 385411 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/extensions/AbuseFilter@master] Remove coding standard rule exclusions

https://gerrit.wikimedia.org/r/385411

Change 385411 abandoned by Huji:
Remove coding standard rule exclusions

Reason:
Per MaxSem

https://gerrit.wikimedia.org/r/385411

Change 385410 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add missing documentation for protected functions

https://gerrit.wikimedia.org/r/385410

Change 424140 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Fix coding conventions exclusion rules

https://gerrit.wikimedia.org/r/424140

Change 424140 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Fix coding conventions exclusion rules

https://gerrit.wikimedia.org/r/424140

@Daimona thanks for working on this. It seems like the only item left is the use of $wgTitle which only in the filterAction() function in AbuseFilter.php and I honestly don't understand why this is necessary.

We know from https://phabricator.wikimedia.org/T55498 that the latter part of the function (where the change to $wgTitle is undone) is necessary if the first part is in place. But I am not convinced why we have to override $wgTitle to begin with. It seems like we only do it when $wgTitle is empty, and in that case we change it to the Title object representing Special:AbuseFilter but then again filterAction() is supposed to just run the actions of a filter (right?) and I don't think there is any filter that can run any action when a page is visiting Special:AbuseFilter (edits to abuse filters don't trigger AbuseFilter itself). What am I missing here?

@Huji I left wgTitle in place exactly because I couldn't really understand the way it's used. Maybe when logging in with CentralAuth you trigger a 'createaccount' action, filters are executed, but you weren't visiting a specific page and so the title is overridden? I really don't know.

OH WAIT: After writing the line above, I found the culprit: https://phabricator.wikimedia.org/rEABF357c4f030e0fdfaa5c4338c0e3100e1b2e69daca. So we override wgTitle for API calls. I can't use APIs really well, but I think we should provide a cleaner workaround: find where the API needs the title and, only there, provide a bogus one. Unfortunately, I can't tell where wgTitle is used in API calls. In any case, if there isn't an alternative solution, I think that replacing wgTitle with mw.config won't break anything.

@werdna that is something you had added in 2009. Do you have any insight as to when an API call won't come with a title?

I'm afraid he won't answer :-) He doesn't work for WMF anymore and that phab account is unused since a couple of years. Anyway, I'll do some experiments with APIs trying to figure out where wgTitle is necessary, and if there's a way to remove it.

I tried inserting a wfDebug inside the if clause and then started to run APIs searching the debug log. Result: none of my tests (about 15-20) triggered the if. Also, since the commit title is quite cryptic (and no description is provided), it's not really clear whether it's intended for AF's own APIs (but I couldn't find any in 2009's code) or any other API call (maybe something related to CentralAuth per T55498?). As far as we know, wgTitle may as well be not needed anymore. I'll send a patch to use mw.config, also adding a wfdebug to the if, so that if someone notices it we may finally know when that situation happens.

No, no no. By chance, I found https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/5924eb85f03b850017c66f01eed24855e31378db%5E%21/api.php. This commit, filed back in 2009, fixes the problem of null wgTitle directly inside api.php. This means that our internal workaround isn't needed anymore, so I'll directly remove anything involving wgTitle.

Change 428045 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Remove unused wgTitle + remove exception from PHPCS

https://gerrit.wikimedia.org/r/428045

Change 428045 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Remove unused wgTitle + remove exception from PHPCS

https://gerrit.wikimedia.org/r/428045

I see all these errors (there are three) are due to @static annotations. I'm not sure what the right fix is:

1. Make them static. If they aren't, there must be a reason.
2. Change the annotation to be a full note, i.e. move the annotation to the function doc (and add a couple of words)
3. Add the annotation as allowed to codesniffer

Change 457364 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Remove PHPCS exclusion and fix it

https://gerrit.wikimedia.org/r/457364

In T178007#4513180, @Daimona wrote:

I see all these errors (there are three) are due to @static annotations. I'm not sure what the right fix is:

1. Make them static. If they aren't, there must be a reason.
2. Change the annotation to be a full note, i.e. move the annotation to the function doc (and add a couple of words)
3. Add the annotation as allowed to codesniffer

4. Remove them.

I don't really see any purpose in having @static annotations. The function is static, and we can tell that simply by looking at the function definition.

@Legoktm Gawrsh, I didn't notice that those function already had the "static" keyword. Actually, I was pretty sure they weren't declared as static, and thus my doubts. Removing those annotations in a minute.

Change 457364 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Remove PHPCS exclusion and fix it

https://gerrit.wikimedia.org/r/457364

Change 457376 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Remove the last PHPCS exclusion

https://gerrit.wikimedia.org/r/457376

Change 457376 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Remove the last PHPCS exclusion

https://gerrit.wikimedia.org/r/457376