Add a UNUSED-IGNORELIST lint for unused lines in lint.ignore

Author: gsnedders

lint.ignore

@@ -59,7 +59,6 @@ W3C-TEST.ORG: .gitignore
 ## Documentation ##
 
 W3C-TEST.ORG: README.md
-W3C-TEST.ORG: */README.md
 W3C-TEST.ORG: docs/*
 WEB-PLATFORM.TEST:*/README.md
 WEB-PLATFORM.TEST:docs/*
@@ -87,7 +86,6 @@ CR AT EOL: html/semantics/forms/the-textarea-element/multiline-placeholder-crlf.
 CR AT EOL: html/semantics/forms/the-input-element/multiline-placeholder-cr.html
 CR AT EOL: html/semantics/forms/the-input-element/multiline-placeholder-crlf.html
 CR AT EOL: webvtt/parsing/file-parsing/tests/support/newlines.vtt
-CR AT EOL: css/css-text/ellisize-rtl-text-crash.html
 CR AT EOL: html/syntax/charset/after-head-after-1kb-crlf.html
 CR AT EOL: html/syntax/charset/after-head-in-1kb-crlf.html
 
@@ -173,8 +171,6 @@ SET TIMEOUT: encrypted-media/polyfill/chrome-polyfill.js
 SET TIMEOUT: encrypted-media/polyfill/clearkey-polyfill.js
 SET TIMEOUT: encrypted-media/scripts/playback-temporary-events.js
 SET TIMEOUT: fedcm/support/fedcm-helper.sub.js
-SET TIMEOUT: fedcm/support/fedcm-iframe.html
-SET TIMEOUT: fedcm/support/fedcm/disconnect-iframe.html
 SET TIMEOUT: fedcm/support/login_delay.html
 SET TIMEOUT: fetch/fetch-later/resources/fetch-later-helper.js
 SET TIMEOUT: fetch/metadata/resources/helper.sub.js
@@ -187,7 +183,6 @@ SET TIMEOUT: focus/support/iframe-contentwindow-focus-with-different-site-interm
 SET TIMEOUT: focus/support/iframe-contentwindow-focus-with-different-site-intermediate-frame-middle.sub.html
 SET TIMEOUT: focus/support/iframe-focuses-parent-different-site-inner.html
 SET TIMEOUT: focus/support/iframe-focuses-parent-same-site-inner.html
-SET TIMEOUT: generic-sensor/resources/iframe_sensor_handler.html
 SET TIMEOUT: html/browsers/browsing-the-web/back-forward-cache/resources/inflight-fetch-helper.js
 SET TIMEOUT: html/browsers/browsing-the-web/history-traversal/*
 SET TIMEOUT: html/browsers/browsing-the-web/navigating-across-documents/*
@@ -228,7 +223,6 @@ SET TIMEOUT: mixed-content/generic/sanity-checker.js
 SET TIMEOUT: navigation-api/navigation-history-entry/entries-after-bfcache-in-iframe.html
 SET TIMEOUT: navigation-timing/*
 SET TIMEOUT: old-tests/submission/Microsoft/history/history_000.htm
-SET TIMEOUT: paint-timing/resources/subframe-painting.html
 SET TIMEOUT: performance-timeline/resources/navigation-id-detached-frame-page.html
 SET TIMEOUT: png/apng/acTL-plays-one.html
 SET TIMEOUT: png/apng/acTL-plays-two.html
@@ -306,7 +300,6 @@ SET TIMEOUT: web-animations/crashtests/reparent-animating-element-002.html
 SET TIMEOUT: web-animations/timing-model/animations/*
 SET TIMEOUT: web-locks/crashtests/after-worker-termination.https.html
 SET TIMEOUT: webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/mediaElementAudioSourceToScriptProcessorTest.html
-SET TIMEOUT: webauthn/*timeout.https.html
 SET TIMEOUT: webdriver/*
 SET TIMEOUT: webmessaging/*
 SET TIMEOUT: webrtc-encoded-transform/script-metadata-transform-worker.js
@@ -324,15 +317,13 @@ GENERATE_TESTS: resources/test/tests/functional/generate-callback.html
 GENERATE_TESTS: resources/testharness.js
 
 # generate_tests usage (should be got rid of)
-GENERATE_TESTS: html/canvas/element/drawing-images-to-the-canvas/*
 GENERATE_TESTS: html/canvas/element/manual/drawing-images-to-the-canvas/*
 GENERATE_TESTS: css/css-shapes/shape-outside/values/*
 GENERATE_TESTS: css/css-tables/bounding-box-computation-1.html
 GENERATE_TESTS: css/css-tables/bounding-box-computation-2.html
 GENERATE_TESTS: css/css-tables/bounding-box-computation-3.html
 GENERATE_TESTS: css/css-tables/caption-side-1.html
 GENERATE_TESTS: css/css-tables/fixed-layout-1.html
-GENERATE_TESTS: css/css-tables/fixed-layout-2.html
 GENERATE_TESTS: css/css-tables/height-distribution/computing-row-measure-0.html
 GENERATE_TESTS: css/css-tables/height-distribution/computing-row-measure-1.html
 GENERATE_TESTS: css/css-tables/height-distribution/percentage-sizing-of-table-cell-children.html
@@ -342,7 +333,6 @@ GENERATE_TESTS: css/css-tables/html5-table-formatting-1.html
 GENERATE_TESTS: css/css-tables/html5-table-formatting-2.html
 GENERATE_TESTS: css/css-tables/html5-table-formatting-3.html
 GENERATE_TESTS: css/css-tables/html5-table-formatting-fixed-layout-1.html
-GENERATE_TESTS: css/css-tables/table-model-fixup-2.html
 GENERATE_TESTS: css/css-tables/table-model-fixup.html
 GENERATE_TESTS: css/css-tables/visibility-collapse-col-001.html
 GENERATE_TESTS: css/css-tables/visibility-collapse-row-001.html
@@ -419,7 +409,6 @@ SET TIMEOUT: scheduler/tentative/current-task-signal-async-abort.any.js
 SET TIMEOUT: scheduler/tentative/current-task-signal-async-priority.any.js
 SET TIMEOUT: screen-capture/tentative/getdisplaymedia-captured-surface-resolution.https.html
 SET TIMEOUT: speculation-rules/prerender/resources/activation-start.html
-SET TIMEOUT: speculation-rules/prerender/resources/prerender-response-code.html
 SET TIMEOUT: speculation-rules/prerender/resources/prerender-while-prerender-outer.html
 SET TIMEOUT: speculation-rules/prerender/resources/deferred-promise-utils.js
 SET TIMEOUT: speculation-rules/prerender/resources/session-history-harness.js
@@ -447,9 +436,6 @@ SET TIMEOUT: acid/acid3/test.html
 *: resources/.gitignore
 *: webaudio/.gitignore
 
-## Third party data files
-TRAILING WHITESPACE: resources/chromium/*
-
 ## Test plans and implementation reports
 *: css/*/test-plan/*
 
@@ -496,8 +482,6 @@ TRAILING WHITESPACE: css/css-fonts/support/fonts/gsubtest-lookup3.ufo/features.f
 SET TIMEOUT: css/compositing/mix-blend-mode/mix-blend-mode-parent-with-3D-transform-and-transition.html
 SET TIMEOUT: css/compositing/mix-blend-mode/mix-blend-mode-sibling-with-3D-transform-and-transition.html
 SET TIMEOUT: css/css-transitions/events-007.html
-SET TIMEOUT: css/css-transitions/support/generalParallelTest.js
-SET TIMEOUT: css/css-transitions/support/runParallelAsyncHarness.js
 SET TIMEOUT: css/css-transitions/transitioncancel-001.html
 SET TIMEOUT: css/css-values/reference/vh-update-and-transition-in-subframe-ref.html
 SET TIMEOUT: css/css-values/reference/vh-update-and-transition-in-subframe-iframe-ref.html
@@ -545,8 +529,6 @@ SET TIMEOUT: css/CSS2/selectors/dom-hover-002.xht
 SET TIMEOUT: css/CSS2/tables/tables-102.xht
 SET TIMEOUT: css/mediaqueries/min-width-tables-001.html
 SET TIMEOUT: css/css-text/crashtests/rendering-rtl-bidi-override-crash.html
-SET TIMEOUT: css/css-backgrounds/color-mix-currentcolor-border-repaint-parent.html
-SET TIMEOUT: svg/painting/currentcolor-fill-stroke-repaint.html
 SET TIMEOUT: resource-timing/resources/run-async-tasks-promise.js
 
 # CSS tests that used to be at the top level and weren't subject to lints
@@ -611,7 +593,6 @@ MISSING-LINK: css/geometry/*.any.js
 MISSING-LINK: css/filter-effects/*.any.js
 
 # Tests that use WebKit/Blink testing APIs
-LAYOUTTESTS APIS: import-maps/data-driven/resources/test-helper-iframe.js
 LAYOUTTESTS APIS: resources/chromium/enable-hyperlink-auditing.js
 LAYOUTTESTS APIS: resources/chromium/webxr-test.js
 LAYOUTTESTS APIS: webxr/resources/webxr_util.js
@@ -626,7 +607,6 @@ WEB-PLATFORM.TEST:web-bundle/resources/generate-test-wbns.sh
 WEB-PLATFORM.TEST:web-bundle/resources/nested/*.wbn
 WEB-PLATFORM.TEST:web-bundle/resources/wbn/*.wbn
 WEB-PLATFORM.TEST:web-bundle/subresource-loading/*.html
-WEB-PLATFORM.TEST:web-bundle/subresource-loading/resources/*.js
 
 # The /.well-known/shared-stored/trusted-origins file uses hard-coded hosts
 WEB-PLATFORM.TEST:shared-storage/resources/trusted-origins.py
@@ -669,20 +649,16 @@ AHEM SYSTEM FONT: css/css-fonts/font-size-adjust-012-ref.html
 AHEM SYSTEM FONT: css/css-fonts/font-size-adjust-013.html
 AHEM SYSTEM FONT: css/css-fonts/font-size-adjust-013-ref.html
 AHEM SYSTEM FONT: css/css-fonts/font-size-adjust-014.html
-AHEM SYSTEM FONT: css/css-fonts/font-size-adjust-014-ref.html
 AHEM SYSTEM FONT: css/css-fonts/font-size-adjust-metrics-override.html
 AHEM SYSTEM FONT: css/css-fonts/font-size-adjust-metrics-override-ref.html
 AHEM SYSTEM FONT: css/css-fonts/line-gap-override.html
 AHEM SYSTEM FONT: css/css-fonts/matching/font-unicode-presented-as-emoji-outline.html
 AHEM SYSTEM FONT: css/css-fonts/matching/font-unicode-presented-as-emoji-outline-ref.html
 AHEM SYSTEM FONT: css/css-fonts/parsing/font-size-adjust-computed.html
-AHEM SYSTEM FONT: css/css-fonts/size-adjust-unicode-range-system-fallback.html
 AHEM SYSTEM FONT: css/css-text/zwnj-renders-invisible.html
 AHEM SYSTEM FONT: css/css-overflow/line-clamp/block-ellipsis-010.html
 AHEM SYSTEM FONT: css/css-overflow/line-clamp/reference/block-ellipsis-010-ref.html
 AHEM SYSTEM FONT: css/css-overflow/line-clamp/reference/block-ellipsis-010-alt-ref.html
-AHEM SYSTEM FONT: html/dom/render-blocking/remove-attr-unblocks-rendering.optional.html
-AHEM SYSTEM FONT: html/dom/render-blocking/remove-element-unblocks-rendering.optional.html
 
 # TODO: The following should be deleted along with the Ahem web font cleanup
 # PR (https://github.com/web-platform-tests/wpt/pull/18702)
@@ -774,7 +750,6 @@ SET TIMEOUT: editing/crashtests/make-editable-div-inline-and-set-contenteditable
 SET TIMEOUT: editing/crashtests/outdent-across-svg-boundary.html
 SET TIMEOUT: editing/crashtests/textarea-will-be-blurred-by-focus-event-listener.html
 SET TIMEOUT: mathml/crashtests/mozilla/*
-SET TIMEOUT: selection/crashtests/selection-modify-line-boundary-around-empty-details.html
 PARSE-FAILED: mathml/crashtests/mozilla/289180-1.xml
 
 # Invalid HTML syntax /> on non-void elements
@@ -834,9 +809,7 @@ TESTDRIVER-IN-UNSUPPORTED-TYPE: speech-api/SpeechRecognition-phrases-manual.http
 
 # Tests automatically imported from the WebAssembly/spec repository that should be removed after getting fixed upstream.
 SET TIMEOUT: wasm/core/js/harness/testharness.js
-ASSERT_THROWS: wasm/core/js/harness/testharness.js
 GENERATE_TESTS: wasm/core/js/harness/testharness.js
-PROMISE_REJECTS: wasm/core/js/harness/testharness.js
 
 # Legitimate use of test_driver_internal
 TEST DRIVER INTERNAL: resources/testdriver.js

tools/lint/lint.py

@@ -279,6 +279,55 @@ def check_unique_case_insensitive_paths(repo_root: Text, paths: List[Text]) -> L
     return errors
 
 
+def check_unused_ignorelist(repo_root: Text,
+                           paths: List[Text],
+                           ignorelist: Ignorelist,
+                           used_entries: Dict[str, Dict[str, Set[Optional[int]]]]) -> List[rules.Error]:
+    """
+    Check for ignorelist entries that were never used during this lint run.
+    Only reports unused entries if the corresponding file pattern matches
+    files in the currently linted paths.
+
+    :param repo_root: the repository root
+    :param paths: list of currently linted paths
+    :param ignorelist: the parsed ignorelist
+    :param used_entries: dict tracking which ignorelist entries were used
+    :returns: a list of errors for unused ignorelist entries
+    """
+    errors = []
+
+    normalized_paths = {os.path.normcase(path) for path in paths}
+
+    for error_type, file_patterns in ignorelist.items():
+        for file_match, allowed_lines in file_patterns.items():
+            # Allow non-matching patterns like `CR AT EOL: *.png`.
+            if "/" not in file_match and file_match.startswith("*."):
+                continue
+
+            used_allowed_lines = used_entries.get(error_type, {}).get(file_match, set())
+            if used_allowed_lines == allowed_lines:
+                continue
+
+            assert used_allowed_lines.issubset(allowed_lines)
+
+            matches_linted_path = any(
+                fnmatch.fnmatchcase(norm_path, file_match)
+                for norm_path in normalized_paths
+            )
+
+            if not matches_linted_path:
+                continue
+
+            for missing_allowed_line in allowed_lines - used_allowed_lines:
+                line_suffix = "" if missing_allowed_line is None else f":{missing_allowed_line}"
+                errors.append(rules.UnusedIgnorelistEntry.error(
+                    "lint.ignore",
+                    (error_type, file_match + line_suffix)
+                ))
+
+    return errors
+
+
 def parse_ignorelist(f: IO[Text]) -> Tuple[Ignorelist, Set[Text]]:
     """
     Parse the ignorelist file given by `f`, and return the parsed structure.
@@ -315,9 +364,16 @@ def parse_ignorelist(f: IO[Text]) -> Tuple[Ignorelist, Set[Text]]:
     return data, skipped_files
 
 
-def filter_ignorelist_errors(data: Ignorelist, errors: Sequence[rules.Error]) -> List[rules.Error]:
+def filter_ignorelist_errors(data: Ignorelist,
+                            errors: Sequence[rules.Error],
+                            used_entries: Dict[str, Dict[str, Set[Optional[int]]]]) -> List[rules.Error]:
     """
     Filter out those errors that are ignored in `data`.
+
+    :param data: the ignorelist data structure
+    :param errors: the list of errors to filter
+    :param used_entries: dict to track which ignorelist entries were used
+    :returns: filtered list of errors
     """
 
     if not errors:
@@ -335,6 +391,11 @@ def filter_ignorelist_errors(data: Ignorelist, errors: Sequence[rules.Error]) ->
                 if None in allowed_lines or line in allowed_lines:
                     if fnmatch.fnmatchcase(normpath, file_match):
                         skipped[i] = True
+                        used_lines = used_entries.setdefault(error_type, {}).setdefault(file_match, set())
+                        if None in allowed_lines:
+                            used_lines.add(None)
+                        else:
+                            used_lines.add(line)
 
     return [item for i, item in enumerate(errors) if not skipped[i]]
 
@@ -1008,6 +1069,8 @@ def lint(repo_root: Text,
     if ignore_glob:
         skipped_files |= set(ignore_glob)
 
+    used_ignorelist_entries: Dict[str, Dict[str, Set[Optional[int]]]] = {}
+
     output_errors = {"json": output_errors_json,
                      "markdown": output_errors_markdown,
                      "normal": output_errors_text}[output_format]
@@ -1021,7 +1084,7 @@ def process_errors(errors: List[rules.Error]) -> Optional[Tuple[Text, Text]]:
                   a tuple of the error type and the path otherwise
         """
 
-        errors = filter_ignorelist_errors(ignorelist, errors)
+        errors = filter_ignorelist_errors(ignorelist, errors, used_ignorelist_entries)
         if not errors:
             return None
 
@@ -1079,6 +1142,9 @@ def process_errors(errors: List[rules.Error]) -> Optional[Tuple[Text, Text]]:
         errors = check_all_paths(repo_root, paths)
         last = process_errors(errors) or last
 
+    unused_errors = check_unused_ignorelist(repo_root, paths, ignorelist, used_ignorelist_entries)
+    last = process_errors(unused_errors) or last
+
     if output_format in ("normal", "markdown"):
         output_error_count(error_count)
         if error_count:

tools/lint/rules.py

@@ -379,6 +379,12 @@ class MissingTestInWebFeaturesFile(Rule):
     """)
 
 
+class UnusedIgnorelistEntry(Rule):
+    name = "UNUSED-IGNORELIST"
+    description = "Unused ignorelist entry: %s: %s"
+    to_fix = "Remove this entry from lint.ignore if it's no longer needed"
+
+
 EXTENSIONS = {
     "html": [".html", ".htm"],
     "xhtml": [".xht", ".xhtml"],

tools/lint/tests/dummy/lint.ignore

@@ -1 +1,8 @@
+# Totally ignore broken_ignored.html
 *:broken_ignored.html
+
+# used_entry.html has trailing whitespace
+TRAILING WHITESPACE: used_entry.html
+
+# unused_entry.html but file has no errors
+TRAILING WHITESPACE: unused_entry.html

tools/lint/tests/dummy/unused_entry.html

@@ -0,0 +1 @@
+<p>This file has no lint errors.</p>

tools/lint/tests/dummy/used_entry.html

@@ -0,0 +1 @@
+<p>This file has trailing whitespace.</p>   

tools/lint/tests/test_lint.py

@@ -32,23 +32,23 @@ def test_filter_ignorelist_errors():
     filteredfile = 'svg/test.html'
     unfilteredfile = 'html/test.html'
     # Tests for passing no errors
-    filtered = filter_ignorelist_errors(ignorelist, [])
+    filtered = filter_ignorelist_errors(ignorelist, [], {})
     assert filtered == []
-    filtered = filter_ignorelist_errors(ignorelist, [])
+    filtered = filter_ignorelist_errors(ignorelist, [], {})
     assert filtered == []
     # Tests for filtering on file and line number
-    filtered = filter_ignorelist_errors(ignorelist, [['CONSOLE', '', filteredfile, 12]])
+    filtered = filter_ignorelist_errors(ignorelist, [['CONSOLE', '', filteredfile, 12]], {})
     assert filtered == []
-    filtered = filter_ignorelist_errors(ignorelist, [['CONSOLE', '', unfilteredfile, 12]])
+    filtered = filter_ignorelist_errors(ignorelist, [['CONSOLE', '', unfilteredfile, 12]], {})
     assert filtered == [['CONSOLE', '', unfilteredfile, 12]]
-    filtered = filter_ignorelist_errors(ignorelist, [['CONSOLE', '', filteredfile, 11]])
+    filtered = filter_ignorelist_errors(ignorelist, [['CONSOLE', '', filteredfile, 11]], {})
     assert filtered == [['CONSOLE', '', filteredfile, 11]]
     # Tests for filtering on just file
-    filtered = filter_ignorelist_errors(ignorelist, [['INDENT TABS', '', filteredfile, 12]])
+    filtered = filter_ignorelist_errors(ignorelist, [['INDENT TABS', '', filteredfile, 12]], {})
     assert filtered == []
-    filtered = filter_ignorelist_errors(ignorelist, [['INDENT TABS', '', filteredfile, 11]])
+    filtered = filter_ignorelist_errors(ignorelist, [['INDENT TABS', '', filteredfile, 11]], {})
     assert filtered == []
-    filtered = filter_ignorelist_errors(ignorelist, [['INDENT TABS', '', unfilteredfile, 11]])
+    filtered = filter_ignorelist_errors(ignorelist, [['INDENT TABS', '', unfilteredfile, 11]], {})
     assert filtered == [['INDENT TABS', '', unfilteredfile, 11]]
 
 
@@ -363,6 +363,33 @@ def test_ignore_glob(caplog):
             assert "ABSOLUTE-URL-REF" in caplog.text
 
 
+def test_unused_ignorelist_entry(caplog):
+    """Test that unused ignorelist entries are detected."""
+    # This test should detect that the ignorelist entry for unused_entry.html
+    # is not being used (file has no errors)
+    with _mock_lint("check_path") as mocked_check_path:
+        with _mock_lint("check_file_contents") as mocked_check_file_contents:
+            rv = lint(_dummy_repo, ["unused_entry.html"], "normal")
+            assert rv == 1
+            assert mocked_check_path.call_count == 1
+            assert mocked_check_file_contents.call_count == 1
+    assert "UNUSED-IGNORELIST" in caplog.text
+    assert "TRAILING WHITESPACE" in caplog.text
+
+
+def test_used_ignorelist_entry(caplog):
+    """Test that used ignorelist entries are not reported."""
+    # This test should NOT report the ignorelist entry for used_entry.html
+    # because it's actually being used (file has trailing whitespace that is ignored)
+    with _mock_lint("check_path") as mocked_check_path:
+        with _mock_lint("check_file_contents") as mocked_check_file_contents:
+            rv = lint(_dummy_repo, ["used_entry.html"], "normal")
+            assert rv == 0
+            assert mocked_check_path.call_count == 1
+            assert mocked_check_file_contents.call_count == 1
+    assert caplog.text == ""
+
+
 def test_all_filesystem_paths():
     with mock.patch(
             'tools.lint.lint.walk',