Bug 1756504 - Remove cpp-virtual-final linter, a minor style check that doesn't diagnose real bugs. r=firefox-static-analysis-reviewers,sylvestre

In bug 1436263, I added a cpp-virtual-final.yml linter to warn about virtual function declarations that included more than one virtual function specifier `virtual`, `final`, or `override`.

I think we should remove this linter now because:

* It's just a style check and doesn't diagnose a real bug. Including more than one virtual function specifier (`virtual`, `final`, or `override`) is harmless and unambiguous, just unnecessary extra code.
* It has caused some engineer frustration because this style check caused their changeset to be backed out of autoland. Backing out and fixing these style issues are not a good use of sheriffs' or engineers' time.
* It doesn't catch all virtual/final/override style issues because:
  * It can't analyze virtual function definitions that span multiple lines.
  * It doesn't check for `virtual void Foo() override` because  there are over 6000 cases already, so our code will never follow this style check consistently.

Differential Revision: https://phabricator.services.mozilla.com/D139454

docs/code-quality/lint/linters/cpp-virtual-final.rst

@@ -1,29 +0,0 @@
-cpp virtual final
-=================
-
-This linter detects the virtual function declarations with multiple specifiers.
-
-It matches our coding style:
-Method declarations must use at most one of the following keywords: virtual, override, or final.
-
-As this linter uses some simple regular expression, it can miss some declarations.
-
-Run Locally
------------
-
-This linter can be run using mach:
-
-.. parsed-literal::
-
-    $ mach lint --linter cpp-virtual-final <file paths>
-
-
-Configuration
--------------
-
-This linter is enabled on all C family files.
-
-Sources
--------
-
-* `Configuration (YAML) <https://searchfox.org/mozilla-central/source/tools/lint/cpp-virtual-final.yml>`_

taskcluster/ci/source-test/mozlint.yml

@@ -81,19 +81,6 @@ rejected-words:
             - '**/*.ftl'
             - 'tools/lint/rejected-words.yml'
 
-cpp-virtual-final:
-    description: lint C++ virtual function declarations
-    treeherder:
-        symbol: cpp(Cvf)
-    run:
-        mach: lint -v -l cpp-virtual-final -f treeherder -f json:/builds/worker/mozlint.json *
-    when:
-        files-changed:
-            - '**/*.cpp'
-            - '**/*.h'
-            - '**/*.mm'
-            - 'tools/lint/cpp-virtual-final.yml'
-
 eslint:
     description: JS lint check
     treeherder:

taskcluster/test/data/bugbug-push-schedules.json

@@ -3845,7 +3845,6 @@
     "test-android-em-7.0-x86_64/opt-geckoview-web-platform-tests-e10s-6",
     "test-windows10-64-2004/debug-reftest-e10s-1",
     "test-windows10-64-2004/opt-web-platform-tests-e10s-4",
-    "source-test-mozlint-cpp-virtual-final",
     "test-vismet-android-hw-g5-7-0-arm7-shippable/opt-browsertime-tp6m-geckoview-cold-bing",
     "test-linux64-shippable/opt-raptor-tp6-firefox-cold-youtube-e10s",
     "test-linux64-shippable-qr/opt-raptor-tp6-firefox-cold-twitch-e10s",

taskcluster/test/params/mb-promote-devedition-partials.yml

@@ -312,7 +312,6 @@ existing_tasks:
     source-test-file-metadata-bugzilla-components: elkfO6TuSzm0LeBqPrmLnA
     source-test-mozlint-clang-format: B3p5yAV1QX-pRvlkWHVpRw
     source-test-mozlint-codespell: ZCS4EJ-YRqGyFTlWTYLhSQ
-    source-test-mozlint-cpp-virtual-final: Frizz4o-S6W2VraOUoxXvg
     source-test-mozlint-eslint: CyO_GkCrTZGE6s9516LeUQ
     source-test-mozlint-file-perm: IZZs_nU-QBWacV1XtjHulw
     source-test-mozlint-file-whitespace: RZSO9OqlTJOWRZGcvCc-kA

taskcluster/test/params/mb-ship-geckoview.yml

@@ -342,7 +342,6 @@ existing_tasks:
     source-test-mozlint-android-lints: VD_PgkC1Q6qJeH1uPLAsjA
     source-test-mozlint-clippy: aNh7FWCNRwS6pWXUHarX_g
     source-test-mozlint-codespell: RU3VkeDaTSuwb6mxScgySQ
-    source-test-mozlint-cpp-virtual-final: LEaGXavZTM-frKkoQg-HEw
     source-test-mozlint-eslint: RElFH_NuRIaPeAmiI51M5A
     source-test-mozlint-file-perm: T-KF4y82TvSn3LGymzNKDQ
     source-test-mozlint-file-whitespace: FLIKkIX0Te6fahJdLAx_Qg

taskcluster/test/params/mc-ship-geckoview.yml

@@ -342,7 +342,6 @@ existing_tasks:
     source-test-mozlint-android-lints: VD_PgkC1Q6qJeH1uPLAsjA
     source-test-mozlint-clippy: aNh7FWCNRwS6pWXUHarX_g
     source-test-mozlint-codespell: RU3VkeDaTSuwb6mxScgySQ
-    source-test-mozlint-cpp-virtual-final: LEaGXavZTM-frKkoQg-HEw
     source-test-mozlint-eslint: RElFH_NuRIaPeAmiI51M5A
     source-test-mozlint-file-perm: T-KF4y82TvSn3LGymzNKDQ
     source-test-mozlint-file-whitespace: FLIKkIX0Te6fahJdLAx_Qg

taskcluster/test/params/mr-ship-geckoview.yml

@@ -342,7 +342,6 @@ existing_tasks:
     source-test-mozlint-android-lints: VD_PgkC1Q6qJeH1uPLAsjA
     source-test-mozlint-clippy: aNh7FWCNRwS6pWXUHarX_g
     source-test-mozlint-codespell: RU3VkeDaTSuwb6mxScgySQ
-    source-test-mozlint-cpp-virtual-final: LEaGXavZTM-frKkoQg-HEw
     source-test-mozlint-eslint: RElFH_NuRIaPeAmiI51M5A
     source-test-mozlint-file-perm: T-KF4y82TvSn3LGymzNKDQ
     source-test-mozlint-file-whitespace: FLIKkIX0Te6fahJdLAx_Qg

tools/lint/cpp-virtual-final.yml

@@ -1,25 +0,0 @@
----
-cpp-virtual-final:
-    description: "Virtual function declarations should specify only one of
-                  `virtual`, `final`, or `override`"
-    level: error
-    include: ['.']
-    extensions: ['cc', 'cpp', 'h', 'mm']
-    type: regex
-    #
-    # This lint warns about:
-    #
-    #   virtual void Bad1() final
-    #   void Bad2() final override
-    #   void Bad3() override final
-    #
-    # Caveats: This lint ...
-    #
-    #   * Doesn't warn about `virtual void NotBad() override` at this time
-    #     because there are currently 6963 instances of this pattern.
-    #
-    #   * Doesn't warn about function declarations that span multiple lines
-    #     because the regex can't match across line breaks.
-    #
-    #           virtual    )    final  |  final  override  |  override  final
-    payload: ^ *virtual .+\).+\bfinal\b|\bfinal +override\b|\boverride +final\b

xpcom/base/nsCycleCollectionParticipant.h

@@ -686,11 +686,8 @@ T* DowncastCCParticipant(void* aPtr) {
  * builds.
  */
 #ifdef DEBUG
-// clang-format off
-// Force this line to remain this way to make sure we don't trigger the
-// lint cpp-virtual-final. see bug 1505943
-#define NOT_INHERITED_CANT_OVERRIDE virtual void BaseCycleCollectable() final {}
-// clang-format on
+#  define NOT_INHERITED_CANT_OVERRIDE \
+    virtual void BaseCycleCollectable() final {}
 #else
 #  define NOT_INHERITED_CANT_OVERRIDE
 #endif