From 2fffde08916a0b1ca74075ab0d482fafcf83245f Mon Sep 17 00:00:00 2001 From: Rubidium Date: Thu, 2 Mar 2023 22:02:37 +0100 Subject: [PATCH] Add: workflow and script for checking missing mode enforcements --- .github/script-missing-mode-enforcement.py | 71 +++++++++++++++++++ .github/unused-strings.py | 7 +- .../script-missing-mode-enforcement.yml | 22 ++++++ 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 .github/script-missing-mode-enforcement.py create mode 100644 .github/workflows/script-missing-mode-enforcement.yml diff --git a/.github/script-missing-mode-enforcement.py b/.github/script-missing-mode-enforcement.py new file mode 100644 index 0000000000..5903b5f0a0 --- /dev/null +++ b/.github/script-missing-mode-enforcement.py @@ -0,0 +1,71 @@ +""" +Script to scan the OpenTTD's script API for functions that miss checks for the +function being called from the right mode (deity or company mode). + +When a function calls either ScriptObject::Command or ScriptObject::GetCompany +then the function is considered dangerous. When one of the mode enforcement +macros from script_error.hpp, i.e. EnforceDeityMode, EnforceCompanyModeValid or +EnforceDeityOrCompanyModeValid, are called in the function, then we consider +that the function has mode enforcement. + +Any dangerous function for which no enforcement is found are emitted as errors. +""" + +import glob +import re +import sys + + +def check_mode_enforcement(path): + errors = [] + with open(path, "r") as reader: + mode_enforcement_found = False + dangerous_function = False + for line in reader: + # Line does not start with a tab and have ::. That looks like the begin of a function, so reset the state. + if re.match(r"^[^\t].*\w::\w", line): + mode_enforcement_found = False + dangerous_function = False + currentFunction = line + continue + + if re.match( + r"\t(EnforceDeityMode|EnforceCompanyModeValid|EnforceDeityOrCompanyModeValid|EnforceDeityOrCompanyModeValid_Void)\(", + line, + ): + # Mode enforcement macro found + mode_enforcement_found = True + continue + + if re.match(r".*(ScriptObject::Command|ScriptObject::GetCompany).*", line): + # Dangerous function found + dangerous_function = True + continue + + # Line with only a closing bracket. That looks like the end of a function, so check for the dangerous function without mode enforcement + if re.match(r"^}$", line) and dangerous_function and not mode_enforcement_found: + function_name = currentFunction.rstrip("\n").replace("/* static */ ", "") + errors.append(f"{path}: {function_name}") + + return errors + + +def main(): + errors = [] + for path in sorted(glob.glob("src/script/api/*.cpp")): + # Skip a number of files that yield only false positives + if path.endswith(("script_object.cpp", "script_companymode.cpp", "script_controller.cpp", "script_game.cpp")): + continue + + errors.extend(check_mode_enforcement(path)) + + if errors: + print("Mode enforcement was expected in the following files/functions:") + print("\n".join(errors)) + sys.exit(1) + + print("OK") + + +if __name__ == "__main__": + main() diff --git a/.github/unused-strings.py b/.github/unused-strings.py index 210212c397..527a938dc5 100644 --- a/.github/unused-strings.py +++ b/.github/unused-strings.py @@ -210,11 +210,10 @@ def main(): errors.append(f"ERROR: {string} is (possibly) no longer needed.") if errors: - for error in errors: - print(error) + print("\n".join(errors)) sys.exit(1) - else: - print("OK") + + print("OK") if __name__ == "__main__": diff --git a/.github/workflows/script-missing-mode-enforcement.yml b/.github/workflows/script-missing-mode-enforcement.yml new file mode 100644 index 0000000000..9b9e8e7efa --- /dev/null +++ b/.github/workflows/script-missing-mode-enforcement.yml @@ -0,0 +1,22 @@ +name: Script missing mode enforcement + +on: + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.ref != 'refs/heads/master' }} + +jobs: + script-missing-mode-enforcement: + name: Script missing mode enforcement + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Check for finding script functions that require company/deity mode enforcement/checks + run: | + set -ex + python3 .github/script-missing-mode-enforcement.py