From 9fe658037fcbe36fc0ac77b43a3c7aa241cc0277 Mon Sep 17 00:00:00 2001 From: Vlad Khramov Date: Sat, 12 Apr 2025 13:23:54 +0100 Subject: [PATCH] Inventory of removed error codes to prevent reuse --- .circleci/config.yml | 9 +- scripts/error_codes.py | 226 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 222 insertions(+), 13 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 09b1281c5630..99fcba2640f5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -945,7 +945,14 @@ jobs: - checkout - run: name: Check for error codes - command: scripts/error_codes.py --check + command: | + scripts/error_codes.py --check + # for PRs check that all removed error IDs were added to the inventory + if [[ -n "$CIRCLE_PULL_REQUEST" ]]; then + PR_TARGET_BRANCH="origin/develop" + export PR_TARGET_BRANCH + scripts/error_codes.py --check-removed + fi - matrix_notify_failure_unless_pr chk_pylint: diff --git a/scripts/error_codes.py b/scripts/error_codes.py index 71206ac43f8f..c7348930e46b 100755 --- a/scripts/error_codes.py +++ b/scripts/error_codes.py @@ -5,10 +5,139 @@ import getopt import sys from os import path +import tempfile +import subprocess ENCODING = "utf-8" SOURCE_FILE_PATTERN = r"\b\d+_error\b" +# default target branch for pull requests +PR_TARGET_DEFAULT_BRANCH = 'origin/develop' + +# Error IDs that were previously used but have been removed from the code. They should not be reused. +# For each ID, a file and a removal commit are provided. +REMOVED_IDS = { + "2314", # liblangutil/ParserBase.cpp, 6965f199fd11cada51c5a97ceb72cc5e14534a17 + "6635", # liblangutil/ParserBase.cpp, 6965f199fd11cada51c5a97ceb72cc5e14534a17 + # liblangutil/ParserBase.cpp, 9adbced98e49588bcc29e181f2560c2eadaa22ac + "1957", # liblangutil/ParserBase.cpp, 6965f199fd11cada51c5a97ceb72cc5e14534a17 + # liblangutil/ParserBase.cpp, 9adbced98e49588bcc29e181f2560c2eadaa22ac + "5883", # libsolidity/analysis/OverrideChecker.cpp, 1d5350e32f04e991dbfc8fca402cbc8c7930e85d + "3881", # libsolidity/analysis/ReferencesResolver.cpp, 66a8c7d1ab5b744736d1355ba46c9e1d5f090ac8 + "6546", # libsolidity/analysis/ReferencesResolver.cpp, 66a8c7d1ab5b744736d1355ba46c9e1d5f090ac8 + "8532", # libsolidity/analysis/ReferencesResolver.cpp, 66a8c7d1ab5b744736d1355ba46c9e1d5f090ac8 + # libsolidity/analysis/DocStringAnalyser.cpp, cb5bfc7436ca11bdfa2f0543a3611079e3f4cbee + # libsolidity/analysis/DocStringAnalyser.cpp, d7899a31afd8e1132ce362c522e8a47c17fd3832 + "1093", # libsolidity/analysis/TypeChecker.cpp, 936ea6f950ffd9b4d4bfaa9cd5ae949be0df58fd + # libsolidity/formal/SMTEncoder.cpp, 11a7763f492411d1fb77841caa48fd918dca64d4 + "6706", # libsolidity/analysis/TypeChecker.cpp, 936ea6f950ffd9b4d4bfaa9cd5ae949be0df58fd + "4035", # libsolidity/analysis/TypeChecker.cpp, 936ea6f950ffd9b4d4bfaa9cd5ae949be0df58fd + "7569", # libyul/AsmAnalysis.cpp, 011f8a462d718f3034e72025b1d3d3916faa474d + "9595", # libyul/AsmAnalysis.cpp, 011f8a462d718f3034e72025b1d3d3916faa474d + "9222", # libsolidity/parsing/DocStringParser.cpp, acd42a08c1b1fe0f69dbcd9c97ac75cc0b1b352b + "7079", # libyul/AsmAnalysis.cpp, d211a45aa4844821e03c38b1abcecb05dafd27fe + "4316", # libyul/AsmAnalysis.cpp, 67ebb206eab4863d47f67ed219ee4dbaa985819f + "2450", # libyul/AsmAnalysis.cpp, 9820df58abfe918c5f92c6d4fa066e2da5c3bf35 + "7816", # libsolidity/analysis/DocStringAnalyser.cpp, cb5bfc7436ca11bdfa2f0543a3611079e3f4cbee + # libsolidity/analysis/DocStringAnalyser.cpp, d7899a31afd8e1132ce362c522e8a47c17fd3832 + "5380", # libsolidity/analysis/TypeChecker.cpp, d41eaeba5686828b85279057f3a7da8be0f9a8f9 + "3312", # libsolidity/analysis/TypeChecker.cpp, d41eaeba5686828b85279057f3a7da8be0f9a8f9 + "3442", # libsolidity/analysis/TypeChecker.cpp, d41eaeba5686828b85279057f3a7da8be0f9a8f9 + "9239", # libsolidity/analysis/TypeChecker.cpp, da36400576304bc6daa924d40684b6655914e4a5 + "9054", # libsolidity/analysis/TypeChecker.cpp, da36400576304bc6daa924d40684b6655914e4a5 + "9155", # libsolidity/analysis/ReferencesResolver.cpp, fc2e9ec2ff4ca397f0aa743cc44b40352894fec2 + "7698", # libsolidity/parsing/Parser.cpp, f73b25bb78e64a57e30ade9c7982d9c2375a3b7b + "6963", # libsolidity/analysis/TypeChecker.cpp, 93c792c696b2929da2b6e2eea31a4c5f60e9188c + "3478", # libsolidity/analysis/TypeChecker.cpp, 93c792c696b2929da2b6e2eea31a4c5f60e9188c + "7439", # libsolidity/parsing/Parser.cpp, 93c792c696b2929da2b6e2eea31a4c5f60e9188c + "4626", # libsolidity/analysis/TypeChecker.cpp, 93c792c696b2929da2b6e2eea31a4c5f60e9188c + "7059", # libsolidity/parsing/Parser.cpp, 93c792c696b2929da2b6e2eea31a4c5f60e9188c + "1719", # libsolidity/analysis/TypeChecker.cpp, 93c792c696b2929da2b6e2eea31a4c5f60e9188c + "6983", # libsolidity/analysis/TypeChecker.cpp, 93c792c696b2929da2b6e2eea31a4c5f60e9188c + "9843", # libsolidity/analysis/DocStringTagParser.cpp, 8b7567f963e00ef15fd5ae97c310064658612ae9 + "1147", # libsolidity/formal/CHC.cpp, 694ec92688617a876d1f1d97beba6ca1b52e6906 + "3672", # libyul/AsmAnalysis.cpp, 291c00c3decd89f3338777ce8836a974b216cd2a + "6493", # libsolidity/analysis/DeclarationTypeChecker.cpp, 5394435bea4e553a86f872b6b2512c50bdef1628 + "4794", # libsolidity/analysis/ReferencesResolver.cpp, ffdb0e37ff788afdf54ea1224a8c8184043322f9 + "9440", # libsolidity/analysis/DocStringTagParser.cpp, 0b45168bcbf5ed5874668f21a3c3d1476cab5620 + "3299", # libsolidity/analysis/SyntaxChecker.cpp, ad311fae1902cf3159834de6d494b46386d0cede + "1054", # liblangutil/ParserBase.cpp, 0ee4a85a841f6494c16a93bd60f3a9ec1a7f2c6e + "9118", # libsolidity/formal/SMTEncoder.cpp, e61b731647b88fe97ee648c4035ecb0fe210eaf0 + "8182", # libsolidity/formal/SMTEncoder.cpp, bd0c46abf55cd6d3beae62bd1618ad8463b4e88b + "6191", # libsolidity/formal/SMTEncoder.cpp, bd0c46abf55cd6d3beae62bd1618ad8463b4e88b + "9056", # libsolidity/formal/SMTEncoder.cpp, bd0c46abf55cd6d3beae62bd1618ad8463b4e88b + "2683", # libsolidity/formal/SMTEncoder.cpp, bd0c46abf55cd6d3beae62bd1618ad8463b4e88b + "9599", # libsolidity/formal/SMTEncoder.cpp, bd0c46abf55cd6d3beae62bd1618ad8463b4e88b + "3408", # libsolidity/analysis/TypeChecker.cpp, e7a6534d4f34e1333bc34f17031997e495fc7f90 + "2332", # libsolidity/analysis/TypeChecker.cpp, e7a6534d4f34e1333bc34f17031997e495fc7f90 + "9149", # libsolidity/formal/SMTEncoder.cpp, 79f550dba94d5831be264745012cf0433556d8fb + "7989", # libsolidity/formal/SMTEncoder.cpp, df8c6d94e30b8c66785e8c77c47a3b080270d194 + "2923", # libsolidity/formal/SMTEncoder.cpp, c8cc73c80c994f84563ac474a455521d38fbcc8f + "5084", # libsolidity/formal/SMTEncoder.cpp, fedbea46cda1d67f00212e2d3ca51e34b8061076 + "2177", # libsolidity/formal/SMTEncoder.cpp, e23d8f559370f5aa740782d22048a1d16fd6e4aa + "1665", # libsolidity/analysis/DeclarationTypeChecker.cpp, 3f14c904b08a358fe1b039519445439eb37d8f37 + "1273", # libsolidity/analysis/TypeChecker.cpp, 3f14c904b08a358fe1b039519445439eb37d8f37 + "6084", # libsolidity/formal/BMC.cpp, 07427c798c48061c608b1fded06c12bd296fa563 + "2370", # libsolidity/analysis/TypeChecker.cpp, 2665eaa4fac3d249fa3d892487ed7b5cfd562e49 + "4639", # libsolidity/formal/SMTEncoder.cpp, 80d743426f7c7da5e55423f6a3223145bf75b559 + "6151", # libsolidity/analysis/TypeChecker.cpp, fda352094f0e9f586004dda48daef9788068f523 + "3978", # libsolidity/analysis/TypeChecker.cpp, fda352094f0e9f586004dda48daef9788068f523 + "3625", # libsolidity/analysis/TypeChecker.cpp, 52c49aebe80ada117f3244e73eb5e643bbbfafe1 + "7885", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "3876", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "9011", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "3263", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "7186", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "3682", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "1950", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "9551", # libsolidity/formal/SMTEncoder.cpp, fa561dbd0e839ce5e198059a0eff139e5860ea89 + "7645", # libsolidity/formal/SMTEncoder.cpp, 0f3924186ea02f3e335c05940dd8c16d1fb783d1 + "1123", # libsolidity/analysis/TypeChecker.cpp, fd9050614a6089168bd4565f4b49c64d04a0ef71 + "1220", # libsolidity/analysis/TypeChecker.cpp, fd9050614a6089168bd4565f4b49c64d04a0ef71 + "7653", # libsolidity/analysis/TypeChecker.cpp, fd9050614a6089168bd4565f4b49c64d04a0ef71 + "9390", # libsolidity/analysis/TypeChecker.cpp, fd9050614a6089168bd4565f4b49c64d04a0ef71 + "2657", # libyul/AsmAnalysis.cpp, ded5d721d2c97170905a536b1f996b79045c84d1 + "5073", # libsolidity/analysis/NameAndTypeResolver.cpp, efe319998110068b8bd3152230a6ff5d0a2339f1 + "9085", # libsolidity/analysis/TypeChecker.cpp, f766700000e1c50f400581fc647f71daef2bb588 + "8261", # libsolidity/analysis/NameAndTypeResolver.cpp, 78a097a012c8abd42b29b4dd2d38bf16a5b7cdc1 + "4224", # libsolidity/analysis/TypeChecker.cpp, e590a99f399e0be33dfba0ab63065d7f17e1e870 + "4579", # libsolidity/analysis/TypeChecker.cpp, e590a99f399e0be33dfba0ab63065d7f17e1e870 + "3772", # libyul/AsmParser.cpp, f04adde6641b09c8744fb1576eda87657b4d9a2d + "2837", # libsolidity/parsing/Parser.cpp, 39e3da19053fb4d892e1aba2de9ba7a5c225f68f + "6660", # libsolidity/formal/SMTEncoder.cpp, 4e343590635f98f7c1d0973e82f9bafc67e03466 + "8195", # libsolidity/formal/SMTEncoder.cpp, 095d33714086880ded0f2ce2c7a8e4493e379613 + "5256", # libsolidity/analysis/DocStringTagParser.cpp, 354f9d101530d38faaeaaf12382bf95e053e21e3 + "9609", # libsolidity/interface/CompilerStack.cpp, 0e8e4eacd59194f66571e136e635e1921af392e5 + "5798", # libyul/AsmParser.cpp, e5ab68ed71460ad088788ac0bc6d94c16832bcfe + "3530", # libsolidity/analysis/DeclarationTypeChecker.cpp, 0004ad876451e28de1b3aa031ab56d08d2cdb3b1 + "4130", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "6672", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "7733", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "2658", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + # libsolidity/experimental/analysis/TypeInference.cpp, fb99132474c177fc861305feeafd43b9cbb93b66 + "2718", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "4599", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "1574", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "3969", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "7484", # libsolidity/analysis/ImmutableValidator.cpp, dad2bf64723cc83f0168bae0310714db01983a50 + "3796", # liblangutil/ParserBase.cpp, 9adbced98e49588bcc29e181f2560c2eadaa22ac + "3347", # liblangutil/ParserBase.cpp, 9adbced98e49588bcc29e181f2560c2eadaa22ac + "3997", # libsolidity/analysis/SyntaxChecker.cpp, 9adbced98e49588bcc29e181f2560c2eadaa22ac + "6156", # libsolidity/formal/SMTEncoder.cpp, 78eb37d2596c9416b3fac9881414bf7e9c524de8 + "6756", # libsolidity/formal/SMTEncoder.cpp, 78eb37d2596c9416b3fac9881414bf7e9c524de8 + "6547", # libsolidity/experimental/analysis/Analysis.cpp, 194b114664c7daebc2ff68af3c573272f5d28913 + "2138", # libsolidity/experimental/analysis/TypeInference.cpp, fb99132474c177fc861305feeafd43b9cbb93b66 + "9817", # libsolidity/experimental/analysis/TypeInference.cpp, fb99132474c177fc861305feeafd43b9cbb93b66 + "4686", # libsolidity/experimental/analysis/TypeInference.cpp, fce70ef0e34b7e6cc8d22b0fc08784758503e8d7 + "6715", # libsolidity/analysis/TypeChecker.cpp, c5685e70544c86f25d87b93048a8bffa963177c6 + "3781", # libyul/AsmAnalysis.cpp, 0d0f2771654b14ee0e8d317a5eb22dbff8eab332 + "9547", # libyul/AsmAnalysis.cpp, 0d0f2771654b14ee0e8d317a5eb22dbff8eab332 + "1733", # libyul/AsmAnalysis.cpp, 0d0f2771654b14ee0e8d317a5eb22dbff8eab332 + "3947", # libyul/AsmAnalysis.cpp, 0d0f2771654b14ee0e8d317a5eb22dbff8eab332 + "5170", # libyul/AsmAnalysis.cpp, 0d0f2771654b14ee0e8d317a5eb22dbff8eab332 + "5622", # libsolidity/formal/BMC.cpp, eb56b4bfe6fe9996342b27d4fc6a4001725d4446 +} + def read_file(file_name): content = None @@ -98,13 +227,12 @@ def fix_ids_in_source_file(file_name, id_to_count, available_ids): print(f"Fixed file: {file_name}") -def fix_ids_in_source_files(file_names, id_to_count): +def fix_ids_in_source_files(file_names, id_to_count, available_ids): """ Fixes ids in given source files; id_to_count contains number of appearances of every id in sources """ - available_ids = {str(error_id) for error_id in range(1000, 10000)} - id_to_count.keys() for file_name in file_names: fix_ids_in_source_file(file_name, id_to_count, available_ids) @@ -123,6 +251,14 @@ def find_files(top_dir, sub_dirs, extensions): return source_file_names +def find_source_files(search_dir): + return find_files( + search_dir, + ["libevmasm", "liblangutil", "libsolc", "libsolidity", "libsolutil", "libyul", "solc"], + [".h", ".cpp"] + ) + + def find_ids_in_test_file(file_name): source = read_file(file_name) pattern = r"^// (.*Error|Warning|Info) \d\d\d\d:" @@ -345,6 +481,68 @@ def examine_id_coverage(top_dir, source_id_to_file_names, new_ids_only=False): return True +def find_ids_in_branch(branch_commit): + """Returns a set of error codes present in the given branch/commit""" + + with tempfile.TemporaryDirectory() as tmp_dir: + # Create a temporary worktree to examine the target branch + subprocess.run( + ['git', 'worktree', 'add', tmp_dir, branch_commit], + check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL + ) + + source_file_names = find_source_files(tmp_dir) + ids = find_ids_in_source_files(source_file_names).keys() + + assert len(ids), "Error IDs weren't found" + + # Clean up the worktree + subprocess.run( + ['git', 'worktree', 'remove', '--force', tmp_dir], + check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL + ) + + return set(ids) + + +def check_removed_error_codes_between_branches(current_ids): + """ + Checks if any error codes were removed between target branch and current branch + but not added to REMOVED_IDS. Returns True if all removed codes are properly tracked. + """ + + # Get the merge base (common ancestor) with the target branch + # If PR_TARGET_BRANCH env var is set (job chk_errorcodes from CI), use that, otherwise use origin/develop + target_branch = os.getenv('PR_TARGET_BRANCH', PR_TARGET_DEFAULT_BRANCH) + try: + merge_base = subprocess.check_output( + ['git', 'merge-base', target_branch, 'HEAD'], + universal_newlines=True + ).strip() + except subprocess.CalledProcessError: + print(f"Error: Could not find merge base with {target_branch}") + return False + + # Get error codes from target branch + try: + target_ids = find_ids_in_branch(merge_base) + except subprocess.CalledProcessError as e: + print(f"Error accessing target branch: {e}") + return False + + # Find removed error codes (present in target but not in current and not in REMOVED_IDS) + removed_ids = target_ids - set(current_ids) - REMOVED_IDS + + if len(removed_ids) > 0: + print(f"Error: The following error codes were removed since {target_branch}:{merge_base} but not added to REMOVED_IDS:") + print_ids(removed_ids) + print("\nPlease add these codes to the REMOVED_IDS set in scripts/error_codes.py") + return False + else: + print(f"No removed IDs found since {merge_base}") + + return True + def main(argv): check = False @@ -352,7 +550,8 @@ def main(argv): no_confirm = False examine_coverage = False next_id = False - opts, _args = getopt.getopt(argv, "", ["check", "fix", "no-confirm", "examine-coverage", "next"]) + check_removed = False + opts, _args = getopt.getopt(argv, "", ["check", "fix", "no-confirm", "examine-coverage", "next", "check-removed"]) for opt, _arg in opts: if opt == "--check": @@ -365,20 +564,22 @@ def main(argv): examine_coverage = True elif opt == "--next": next_id = True + elif opt == "--check-removed": + check_removed = True - if [check, fix, examine_coverage, next_id].count(True) != 1: - print("usage: python error_codes.py --check | --fix [--no-confirm] | --examine-coverage | --next") + if [check, fix, examine_coverage, next_id, check_removed].count(True) != 1: + print("usage: python error_codes.py --check | --fix [--no-confirm] | --examine-coverage | --next | --check-removed") sys.exit(1) cwd = os.getcwd() - source_file_names = find_files( - cwd, - ["libevmasm", "liblangutil", "libsolc", "libsolidity", "libsolutil", "libyul", "solc"], - [".h", ".cpp"] - ) + source_file_names = find_source_files(cwd) source_id_to_file_names = find_ids_in_source_files(source_file_names) + if check_removed: + res = 0 if check_removed_error_codes_between_branches(source_id_to_file_names.keys()) else 1 + sys.exit(res) + ok = True for error_id in sorted(source_id_to_file_names): if len(error_id) != 4: @@ -402,11 +603,12 @@ def main(argv): random.seed() + available_ids = {str(error_id) for error_id in range(1000, 10000)} - source_id_to_file_names.keys() - REMOVED_IDS + if next_id: if not ok: print("Incorrect IDs have to be fixed before applying --next") sys.exit(1) - available_ids = {str(error_id) for error_id in range(1000, 10000)} - source_id_to_file_names.keys() next_id = get_next_id(available_ids) print(f"Next ID: {next_id}") sys.exit(0) @@ -434,7 +636,7 @@ def main(argv): # number of appearances for every id source_id_to_count = { error_id: len(file_names) for error_id, file_names in source_id_to_file_names.items() } - fix_ids_in_source_files(source_file_names, source_id_to_count) + fix_ids_in_source_files(source_file_names, source_id_to_count, available_ids) print("Fixing completed") sys.exit(2)