From f46c2a31f69786f82762fb93043a90cfe25bd5c1 Mon Sep 17 00:00:00 2001
From: Maarten van der Schrieck <maarten@thingsconnected.nl>
Date: Sat, 6 Jan 2024 16:00:51 +0100
Subject: [PATCH] add test/cli/test-json.py: Schema validation for JSON files.

test-json.py uses jsonschema (https://pypi.org/project/jsonschema/) to validate
JSON files against schemas, optionally with custom format checkers.

All JSON files in the project must be accounted for in
test-json.schema_mapping(), either as a schema, or as a file to be validated
against a schema. The JSON schemas are implicitly validated against the most
recent metaschema available in jsonschema.

This commit adds schemas for JSON addon files and for namingng config files.

TODO:
- runtime schema validation
- remove current ad-hoc validation
- generate documentation from schemas
---
 .github/workflows/CI-unixish.yml         |   1 +
 .github/workflows/CI-windows.yml         |   2 +
 .github/workflows/asan.yml               |   1 +
 .github/workflows/tsan.yml               |   1 +
 .github/workflows/ubsan.yml              |   1 +
 addons/addon-namingng-config.schema.json | 141 +++++++++++++++++++++++
 addons/addon.schema.json                 |  35 ++++++
 addons/namingng.config.json              |   5 +-
 test/cli/test-json.py                    | 136 ++++++++++++++++++++++
 test/cli/test-other.py                   |   3 +
 10 files changed, 325 insertions(+), 1 deletion(-)
 create mode 100644 addons/addon-namingng-config.schema.json
 create mode 100644 addons/addon.schema.json
 create mode 100644 test/cli/test-json.py

diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml
index e28f435fd850..5259e0601bbc 100644
--- a/.github/workflows/CI-unixish.yml
+++ b/.github/workflows/CI-unixish.yml
@@ -367,6 +367,7 @@ jobs:
           python3 -m pip install pip --upgrade
           python3 -m pip install pytest
           python3 -m pip install pytest-timeout
+          python3 -m pip install jsonschema
 
       - name: Build cppcheck
         run: |
diff --git a/.github/workflows/CI-windows.yml b/.github/workflows/CI-windows.yml
index 1de7af8d99d8..c67def6ab8a0 100644
--- a/.github/workflows/CI-windows.yml
+++ b/.github/workflows/CI-windows.yml
@@ -134,6 +134,7 @@ jobs:
           python -m pip install pytest || exit /b !errorlevel!
           python -m pip install pytest-custom_exit_code || exit /b !errorlevel!
           python -m pip install pytest-timeout || exit /b !errorlevel!
+          python -m pip install jsonschema || exit /b !errorlevel!
 
       - name: Run CMake
         if: false # TODO: enable
@@ -173,6 +174,7 @@ jobs:
           python -m pytest -Werror --strict-markers -vv test-inline-suppress.py || exit /b !errorlevel!
           python -m pytest -Werror --strict-markers -vv test-more-projects.py || exit /b !errorlevel!
           python -m pytest -Werror --strict-markers -vv test-other.py || exit /b !errorlevel!
+          python -m pytest -Werror --strict-markers -vv test-json.py || exit /b !errorlevel!
           python -m pytest -Werror --strict-markers -vv test-proj2.py || exit /b !errorlevel!
           python -m pytest -Werror --strict-markers -vv test-suppress-syntaxError.py || exit /b !errorlevel!
 
diff --git a/.github/workflows/asan.yml b/.github/workflows/asan.yml
index cc2f4a405e88..3255cb3ca64a 100644
--- a/.github/workflows/asan.yml
+++ b/.github/workflows/asan.yml
@@ -64,6 +64,7 @@ jobs:
           python3 -m pip install pip --upgrade
           python3 -m pip install pytest
           python3 -m pip install pytest-timeout
+          python3 -m pip install jsonschema
 
       # TODO: disable all warnings
       - name: CMake
diff --git a/.github/workflows/tsan.yml b/.github/workflows/tsan.yml
index 8c76c507d0d3..863d0d5b86de 100644
--- a/.github/workflows/tsan.yml
+++ b/.github/workflows/tsan.yml
@@ -64,6 +64,7 @@ jobs:
           python3 -m pip install pip --upgrade
           python3 -m pip install pytest
           python3 -m pip install pytest-timeout
+          python3 -m pip install jsonschema
 
       - name: CMake
         run: |
diff --git a/.github/workflows/ubsan.yml b/.github/workflows/ubsan.yml
index a48fcb4a5013..d123b12f1593 100644
--- a/.github/workflows/ubsan.yml
+++ b/.github/workflows/ubsan.yml
@@ -63,6 +63,7 @@ jobs:
           python3 -m pip install pip --upgrade
           python3 -m pip install pytest
           python3 -m pip install pytest-timeout
+          python3 -m pip install jsonschema
 
       # TODO: disable warnings
       - name: CMake
diff --git a/addons/addon-namingng-config.schema.json b/addons/addon-namingng-config.schema.json
new file mode 100644
index 000000000000..3d166665eabc
--- /dev/null
+++ b/addons/addon-namingng-config.schema.json
@@ -0,0 +1,141 @@
+{
+    "$schema": "https://json-schema.org/draft/2020-12/schema",
+    "$id": "https://cppcheck.com/addon.schema.json",
+    "title": "Addon namingng config",
+    "description": "namingng configuration format",
+    "type": "object",
+    "properties": {
+        "RE_FILE": {
+            "description": "Patterns for filenames",
+            "$ref": "#/definitions/re_list_or_none"
+        },
+        "RE_NAMESPACE": {
+            "description": "Patterns for namespaces",
+            "$ref": "#/definitions/re_list_or_dict_or_none"
+        },
+        "RE_VARNAME": {
+            "description": "Patterns for variable names",
+            "$ref": "#/definitions/re_list_or_dict_or_none"
+        },
+        "RE_PRIVATE_MEMBER_VARIABLE": {
+            "description": "Patterns for private member variable names",
+            "$ref": "#/definitions/re_list_or_dict_or_none"
+        },
+        "RE_PUBLIC_MEMBER_VARIABLE": {
+            "description": "Patterns for public member variable names",
+            "$ref": "#/definitions/re_list_or_dict_or_none"
+        },
+        "RE_GLOBAL_VARNAME": {
+            "description": "Patterns for global variable names",
+            "$ref": "#/definitions/re_list_or_dict_or_none"
+        },
+        "RE_FUNCTIONNAME": {
+            "description": "Patterns for function names",
+            "$ref": "#/definitions/re_list_or_dict_or_none"
+        },
+        "RE_CLASS_NAME": {
+            "description": "Patterns for class names",
+            "$ref": "#/definitions/re_list_or_dict_or_none"
+        },
+        "var_prefixes": {
+            "description": "Variable prefixes per type",
+            "$ref": "#/definitions/prefix_dict"
+        },
+        "function_prefixes": {
+            "description": "Variable prefixes per return type",
+            "$ref": "#/definitions/prefix_dict"
+        },
+        "skip_one_char_variables": {
+            "description": "Whether to ignore one-character local variables (default: false)",
+            "type":"boolean"
+        },
+        "include_guard": {
+            "properties": {
+                "input": {
+                    "description": "What to take as input for the include guard name (default: path)",
+                    "enum": ["path","basename"]
+                },
+                "case": {
+                    "description": "What case the include guard name is in (default: upper)",
+                    "enum": ["upper","lower","keep"]
+                },
+                "prefix": {
+                    "description": "Include guard prefix",
+                    "type": "string"
+                },
+                "suffix": {
+                    "description": "Include guard suffix",
+                    "type": "string"
+                },
+                "max_linenr": {
+                    "description": "Don't consider include guards found after this line number (default: 5)",
+                    "type": "number"
+                },
+                "required": {
+                    "description": "Whether include guards are required for include files (default: true)",
+                    "type": "boolean"
+                },
+                "RE_HEADERFILE": {
+                    "description": "Pattern used to determine whether a file is an include file (default: relative paths ending in .h)",
+                    "$ref": "#/definitions/python_re"
+                }
+            },
+            "additionalProperties": false
+        }
+    },
+    "additionalProperties": false,
+    "required": [],
+    "definitions": {
+        "re_list_or_dict_or_none":{
+            "anyOf": [
+                { "$ref": "#/definitions/re_list" },
+                { "$ref": "#/definitions/re_dict" },
+                { "type": "null" }
+            ]
+        },
+        "re_list_or_none":{
+            "anyOf": [
+                { "$ref": "#/definitions/re_list" },
+                { "type": "null" }
+            ]
+        },
+        "re_list": {
+            "type": "array",
+            "items": {
+                "$ref": "#/definitions/python_re"
+            },
+            "minItems":1
+        },
+        "re_dict": {
+            "type": "object",
+            "propertyNames": {
+                "$ref": "#/definitions/python_re"
+            },
+            "patternProperties": {
+                "": {
+                    "type": "array",
+                    "prefixItems": [
+                        { "type": "boolean" },
+                        { "type": "string" }
+                    ],
+                    "minItems": 2,
+                    "unevaluatedItems": false
+                }
+            }
+        },
+        "prefix_dict": {
+            "type": "object",
+            "patternProperties": {
+                "^[a-zA-Z0-9_*&]+$": {
+                    "type":"string",
+                    "pattern": "^[a-zA-Z0-9_]+$"
+                }
+            },
+            "additionalProperties": false
+        },
+        "python_re": {
+            "type": "string",
+            "format": "python_re"
+        }
+    }
+}
diff --git a/addons/addon.schema.json b/addons/addon.schema.json
new file mode 100644
index 000000000000..9804f75a1618
--- /dev/null
+++ b/addons/addon.schema.json
@@ -0,0 +1,35 @@
+{
+    "$schema": "https://json-schema.org/draft/2020-12/schema",
+    "$id": "https://cppcheck.com/addon.schema.json",
+    "title": "Addon",
+    "description": "Cppcheck addon format",
+    "type": "object",
+    "properties": {
+        "script": {
+            "description": "The Python script containing the addon code.",
+            "type": "string"
+        },
+        "args": {
+            "description": "Arguments to pass to the script.",
+            "type": "array",
+            "items": {
+                "type": "string"
+            },
+            "minItems": 1
+        },
+        "python": {
+            "description": "The full path of the Python interpreter to use.",
+            "type": "string"
+        },
+        "ctu": {
+            "description": "Whether the addon is instructed to perform analysis across multiple translation units.",
+            "type": "boolean"
+        },
+        "executable": {
+            "description": "Override default 'python <script>' invocation [deprecated].",
+            "type": "string"
+        }
+    },
+    "additionalProperties": false,
+    "required": ["script"]
+}
diff --git a/addons/namingng.config.json b/addons/namingng.config.json
index 0abba4f15ac9..9bab20bae97d 100644
--- a/addons/namingng.config.json
+++ b/addons/namingng.config.json
@@ -1,5 +1,8 @@
 {
-    "RE_VARNAME": ["[a-z]*[a-zA-Z0-9_]*\\Z"],
+    "RE_VARNAME": {
+        "[a-z][a-zA-Z0-9_]*\\Z":[false,"must start with a-z"],
+        ".*new":[true,"may not contain 'new'"]
+    },
     "RE_PRIVATE_MEMBER_VARIABLE": null,
     "RE_FUNCTIONNAME": ["[a-z0-9A-Z]*\\Z"],
     "include_guard": {
diff --git a/test/cli/test-json.py b/test/cli/test-json.py
new file mode 100644
index 000000000000..d6aa56292c3b
--- /dev/null
+++ b/test/cli/test-json.py
@@ -0,0 +1,136 @@
+
+# python -m pytest test-json.py
+
+import os
+import sys
+import pytest
+import re
+import json
+import jsonschema
+
+from pathlib import Path
+
+def schema_mapping():
+    def p(*args):
+        return os.path.join(*args)
+    return {
+        p('addons','addon.schema.json'): [
+            p('addons','namingng.json'),
+            'naming.json'
+        ],
+        p('addons','addon-namingng-config.schema.json'): [
+            p('addons','ROS_naming.json'),
+            p('addons','namingng.config.json')
+        ],
+    }
+
+# files that need no validation
+def schema_whitelist():
+    return [
+        'compile_commands.json', # generated during build
+    ]
+
+def __load_json(js,label='JSON'):
+    try:
+        with open(js,'r') as f:
+            return json.load(f)
+    except json.JSONDecodeError as e:
+        assert False,'Failed to parse %s %s at line %d: %s'%(label,js,e.lineno,e.msg)
+    except Exception as e:
+        assert False,'Failed to load %s %s: %s'%(label,js,e)
+
+def validate_json(jsonfile,validator,schema_json_fn,json_fn,verbose=False):
+    test_json = __load_json(jsonfile)
+    issue = None
+    try:
+        validator.validate(instance=test_json)
+    except jsonschema.exceptions.SchemaError as e:
+        issue = 'JSON schema %s did not validate against metaschema'%(schema_json_fn)
+        if verbose:
+            # this will also emit the SchemaError
+            assert False, issue
+    except jsonschema.exceptions.ValidationError as e:
+        issue = 'JSON %s did not validate against schema %s'%(json_fn,schema_json_fn)
+        if verbose:
+            # this will also emit the ValidationError
+            assert False, issue
+    except Exception as e:
+        # In some cases a different exception is raised from deep within jsonschema
+        issue = 'An issue occurred validating %s against %s'%(json_fn,schema_json_fn)
+        if verbose:
+            # this will also emit the unhandled error
+            assert False, issue
+
+    assert issue == None
+
+# we could use @pytest.mark.parametrize('schema', schema_mapping()), to make it
+# one test per schema, BUT that would lead to a lot of double code as we also
+# test whether all JSON files are covered.
+def test_schemas(request):
+    verbose = request.config.option.verbose
+    script_path = Path(os.path.realpath(__file__))
+    assert str(script_path.parents[0].name) == 'cli'
+    assert str(script_path.parents[1].name) == 'test'
+    cppcheck_dir = script_path.parents[2]
+
+    def include(p):
+        # This is a bit clunky but there is no clear separation between
+        # codebase and build/install directories on all CI instances.
+        relpath = p.relative_to(cppcheck_dir)
+        parents = list(relpath.parents)
+        if len(parents)>1 and parents[-2].name in ('bin','build','cmake.output'):
+            return False
+        return True
+
+    json_files_all = set(cppcheck_dir.rglob('*.json'))
+    json_files = [p.resolve() for p in cppcheck_dir.rglob('*.json') if include(p)]
+
+    format_checker = jsonschema.FormatChecker()
+    @format_checker.checks('python_re', AssertionError)
+    def is_python_re(value):
+        if not isinstance(value,str):
+            issue = 'Expect string containing regular expression, got "%s"'%(str(value))
+            if verbose:
+                raise Exception(issue)
+            else:
+                assert False, issue
+        issue = None
+        try:
+            re.compile(value)
+        except re.error as err:
+            issue = 'Error compiling Python regular expression "%s": %s'%(value,str(err))
+        if issue and verbose:
+            # raise an exception describing the issue, so that it is visible if testing --verbose
+            raise Exception(issue)
+        assert issue == None
+        return True
+
+    recent_validator = jsonschema.validators.validator_for(True)
+    validator_factory = jsonschema.validators.create(
+        meta_schema=recent_validator.META_SCHEMA,
+        validators=recent_validator.VALIDATORS
+    )
+
+    for schema_json_fn,jsons in schema_mapping().items():
+        schemafile = cppcheck_dir.joinpath(schema_json_fn)
+        assert schemafile.exists(), 'schema file %s not found'%schema_json_fn
+        json_files.remove(schemafile)
+        schema_json = __load_json(schemafile,'JSON schema')
+        # Note that there is no need to validate the schema against
+        # the metaschema here; this is already done by jsonschema
+        # as a first step in jsonschema.validate().
+
+        validator = validator_factory(schema_json, format_checker=format_checker)
+
+        for json_fn in jsons:
+            jsonfile = cppcheck_dir.joinpath(json_fn)
+            assert jsonfile.exists(), 'JSON file %s not found'%json_fn
+            json_files.remove(jsonfile)
+            validate_json(jsonfile,validator,schema_json_fn,json_fn,verbose=request.config.option.verbose)
+
+    whitelist = schema_whitelist()
+    for f in list(json_files):
+        if f.name in whitelist:
+            json_files.remove(f)
+
+    assert len(json_files)==0, 'JSON files without schema found: %s'%(', '.join([str(p) for p in json_files]))
\ No newline at end of file
diff --git a/test/cli/test-other.py b/test/cli/test-other.py
index 5ed9c4353fd5..c99927bb61d5 100644
--- a/test/cli/test-other.py
+++ b/test/cli/test-other.py
@@ -491,6 +491,9 @@ class _clz {
     assert lines == expect
 
 
+# This can be removed now config is fully validated using a schema.
+# A config with just one fatal issue will suffice to verify that validation is working.
+@pytest.mark.skip
 def test_addon_namingng_config(tmpdir):
     addon_file = os.path.join(tmpdir, 'namingng.json')
     addon_config_file = os.path.join(tmpdir, 'namingng.config.json')