From 584a50d0dc3bf450c91c6cab560656f65b01eeb1 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 15 Mar 2023 18:01:48 -0700 Subject: [PATCH] Fix check-diagnostics for pre-commit (#2683) Unused diagnostics were incorrectly always returning "false" regardless of whether there was an issue. It was still looking for registry file changes, not kind file changes. Also, this should only be run once per pre-commit run (pass_filenames=false). Remove the one unused diagnostic. --- .pre-commit-config.yaml | 4 +++- toolchain/diagnostics/check_diagnostics.py | 6 ++++-- toolchain/diagnostics/diagnostic_kind.def | 3 --- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5b98f6072992..133a2b11e5ed 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -103,8 +103,10 @@ repos: (?x)^( toolchain/.*\.cpp| toolchain/.*\.h| - toolchain/diagnostics/diagnostic_registry\.def + toolchain/diagnostics/check_diagnostics\.py| + toolchain/diagnostics/diagnostic_kind\.def )$ + pass_filenames: false # Run linters last, as formatters and other checks may fix issues. - repo: local diff --git a/toolchain/diagnostics/check_diagnostics.py b/toolchain/diagnostics/check_diagnostics.py index b2154a71defb..4c4db33237dc 100755 --- a/toolchain/diagnostics/check_diagnostics.py +++ b/toolchain/diagnostics/check_diagnostics.py @@ -15,8 +15,8 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import collections from concurrent import futures import itertools -from pathlib import Path import os +from pathlib import Path import re import sys from typing import Dict, List, NamedTuple, Set @@ -98,9 +98,11 @@ def check_uniqueness(uses: Dict[str, List[Location]]) -> bool: def check_unused(decls: Set[str], uses: Dict[str, List[Location]]) -> bool: """If any diagnostic is unused, prints an error and returns true.""" unused = decls.difference(uses.keys()) + if not unused: + return False for diag in sorted(unused): print(f"Unused diagnostic: {diag}") - return False + return True def main() -> None: diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 9dfda228fa1c..ff4bfefa223f 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -84,9 +84,6 @@ CARBON_DIAGNOSTIC_KIND(ExpectedDeclarationName) CARBON_DIAGNOSTIC_KIND(ExpectedDeclarationSemiOrDefinition) CARBON_DIAGNOSTIC_KIND(MethodImplNotAllowed) -// Class and interface diagnostics -CARBON_DIAGNOSTIC_KIND(ExpectedDeducedParam) - // ============================================================================ // Semantics diagnostics // ============================================================================