diff --git a/.gitignore b/.gitignore index f5a79d47..3f0b67ac 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,6 @@ dist/ /logs build .hypothesis/ +.venv/ +.idea/ +.claude/ diff --git a/README.md b/README.md index defe80b8..73ce3d8e 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,122 @@ repos: types: [gdscript] ``` +## Linter plugins + +gdlint supports external plugins for custom lint rules. A plugin is a Python module that exposes a `lint(parse_tree, config)` function and returns a list of `Problem` objects. + +### Example: creating a custom rule + +1. Create a plugin file `no_print_checks.py` in your project root: + +```python +from types import MappingProxyType +from typing import List + +from lark import Tree, Token +from gdtoolkit.linter.problem import Problem +from gdtoolkit.common.utils import get_line, get_column + + +def lint(parse_tree: Tree, config: MappingProxyType) -> List[Problem]: + if "no-print-statement" in config.get("disable", []): + return [] + problems = [] + for call_node in parse_tree.find_data("standalone_call"): + for child in call_node.children: + if isinstance(child, Token) and child.value == "print": + problems.append(Problem( + name="no-print-statement", + description="Avoid using print() in production code", + line=get_line(child), + column=get_column(child), + )) + break + return problems +``` + +The `Problem` dataclass has four fields: `name` (rule identifier — this is what you use to disable the rule), `description` (human-readable message), `line`, and `column`. + +2. Add the plugin to your `gdlintrc` by module name (filename without `.py`). The `plugins` key sits alongside the same settings you already use: + +```yaml +# gdlintrc +class-name: '([A-Z][a-z0-9]*)+' +constant-name: '[A-Z][A-Z0-9]*(_[A-Z0-9]+)*' +max-line-length: 100 +disable: [] +plugins: + - no_print_checks +``` + +3. Run `gdlint` — the plugin rule works exactly like built-in rules: + +``` +$ gdlint my_script.gd +my_script.gd:5: Error: Avoid using print() in production code (no-print-statement) +``` + +### How plugins are loaded + +Plugin names can be either **module names** or **relative paths**: + +```yaml +plugins: + # module name — loaded via importlib, must be on sys.path + - no_print_checks + # relative path — loaded from file relative to cwd (no .py extension) + - ci/gdlint/no_print_checks +``` + +When plugins are configured, the current working directory is automatically added to `sys.path`, so local `.py` files are found by module name. For plugins in subdirectories, use the path form — forward slashes work on all platforms. + +Missing or failing plugins are logged as warnings and skipped — they won't crash the linter. + +### Distributing plugins as packages + +For sharing plugins across projects, structure your plugin as a pip-installable Python package with a `setup.py` or `pyproject.toml`: + +``` +my-gdlint-plugin/ + my_gdlint_plugin/ + __init__.py # must contain lint(parse_tree, config) + some_check.py + setup.py +``` + +Then install and reference the module name in `gdlintrc`: + +``` +pip install my-gdlint-plugin +``` + +```yaml +plugins: + - my_gdlint_plugin +``` + +### Using plugins with pre-commit + +pre-commit runs hooks in isolated virtualenvs, so neither local `.py` files nor globally installed packages are available. Use [`additional_dependencies`](https://pre-commit.com/#config-additional_dependencies) to install plugin packages into the hook's environment. + +`additional_dependencies` accepts anything pip can install — a PyPI package, a git URL, or a local path: + +```yaml +# .pre-commit-config.yaml +repos: + - repo: https://github.com/Scony/godot-gdscript-toolkit + rev: 4.2.2 + hooks: + - id: gdlint + additional_dependencies: + # from PyPI: + - my-gdlint-plugin==1.0.0 + # from a git repo: + - git+https://github.com/your-org/my-gdlint-plugin.git@main + # from a local directory (editable install): + - -e ./linter/my-gdlint-plugin +``` + ## Development [(more)](https://github.com/Scony/godot-gdscript-toolkit/wiki/5.-Development) Everyone is free to fix bugs or introduce new features. For that, however, please refer to existing issue or create one before starting implementation. diff --git a/gdtoolkit/linter/__init__.py b/gdtoolkit/linter/__init__.py index b65e78f0..3bdae4a5 100644 --- a/gdtoolkit/linter/__init__.py +++ b/gdtoolkit/linter/__init__.py @@ -1,4 +1,9 @@ +import importlib +import importlib.util +import logging +import os import re +import sys from collections import defaultdict from types import MappingProxyType from typing import List, Dict, Set @@ -48,13 +53,6 @@ "unnecessary-pass": None, "unused-argument": None, "comparison-with-itself": None, - # not-in-loop (break/continue) # check in godot - # duplicate-argument-name # check in godot - # self-assigning-variable # check in godot - # comparison-with-callable - # duplicate-key # check in godot - # unreachable # check in godot - # using-constant-test # check in godot # class checks "class-definitions-order": [ "tools", @@ -72,15 +70,9 @@ "onreadyprvvars", "others", ], - # useless-super-delegation # design checks - # max-locals "max-returns": 6, - # max-branches - # max-statements - # max-attributes "max-public-methods": 20, - # max-nested-blocks "function-arguments-number": 10, # format checks "max-file-lines": 1000, @@ -92,6 +84,7 @@ "excluded_directories": {".git"}, "no-elif-return": None, "no-else-return": None, + "plugins": [], # never-returning-function # for non-void, typed functions # simplify-boolean-expression # consider-using-in @@ -111,6 +104,22 @@ ) +def _load_plugin(plugin_path: str): + if "/" in plugin_path or "\\" in plugin_path: + normalized = plugin_path.replace("\\", "/") + file_path = os.path.normpath(os.path.join(os.getcwd(), normalized + ".py")) + if not os.path.isfile(file_path): + raise ImportError(f"Plugin file not found: {file_path}") + module_name = os.path.basename(normalized) + spec = importlib.util.spec_from_file_location(module_name, file_path) + if spec is None or spec.loader is None: + raise ImportError(f"Cannot load plugin from {file_path}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + return importlib.import_module(plugin_path) + + def lint_code( gdscript_code: str, config: MappingProxyType = DEFAULT_CONFIG ) -> List[Problem]: @@ -122,6 +131,20 @@ def lint_code( problems += basic_checks.lint(parse_tree, config) problems += misc_checks.lint(parse_tree, config) + plugin_paths = config.get("plugins", []) + if plugin_paths: + cwd = os.getcwd() + if cwd not in sys.path: + sys.path.insert(0, cwd) + for plugin_path in plugin_paths: + try: + module = _load_plugin(plugin_path) + problems += module.lint(parse_tree, config) + except ImportError as exc: + logging.warning("gdlint plugin '%s' not found - skipping (%s)", plugin_path, exc) + except Exception as exc: + logging.warning("gdlint plugin '%s' failed: %s", plugin_path, exc) + problems_to_lines_where_they_are_inactive = _fetch_problem_inactivity_lines( gdscript_code ) diff --git a/tests/linter/common.py b/tests/linter/common.py index 3d78ce22..0afa7648 100644 --- a/tests/linter/common.py +++ b/tests/linter/common.py @@ -22,3 +22,23 @@ def simple_nok_check(code, check_name, line=2, **kwargs): assert len(outcome) == 1 assert outcome[0].name == check_name assert get_line(outcome[0]) == line + + +def multiple_nok_check(code, check_name, lines, **kwargs): + """Check that multiple errors of the same type are found at specified lines.""" + extra_disable = [] if "disable" not in kwargs else kwargs["disable"] + config_w_disable = DEFAULT_CONFIG.copy() + config_w_disable.update({"disable": [check_name] + extra_disable}) + assert len(lint_code(code, config_w_disable)) == 0 + + config = DEFAULT_CONFIG.copy() + config.update({"disable": extra_disable}) + outcome = lint_code(code, config) + assert len(outcome) == len(lines), f"Expected {len(lines)} errors, got {len(outcome)}: {outcome}" + + for problem in outcome: + assert problem.name == check_name + + found_lines = sorted([get_line(p) for p in outcome]) + expected_lines = sorted(lines) + assert found_lines == expected_lines, f"Expected errors on lines {expected_lines}, got {found_lines}" diff --git a/tests/linter/test_basic_checks.py b/tests/linter/test_basic_checks.py index 5d681db0..f74a8202 100644 --- a/tests/linter/test_basic_checks.py +++ b/tests/linter/test_basic_checks.py @@ -31,10 +31,6 @@ '''docstr''' """, """ -func foo(): - await get_tree().create_timer(2.0).timeout -""", -""" func foo(): ('''docstr''') """, diff --git a/tests/linter/test_plugins.py b/tests/linter/test_plugins.py new file mode 100644 index 00000000..b38cef9e --- /dev/null +++ b/tests/linter/test_plugins.py @@ -0,0 +1,129 @@ +import os +import tempfile +import textwrap + +import pytest + +from gdtoolkit.linter import lint_code, DEFAULT_CONFIG + + +def _config_with_plugin(plugin_path): + config = DEFAULT_CONFIG.copy() + config.update({"plugins": [plugin_path]}) + return config + + +PLUGIN_SOURCE = textwrap.dedent("""\ + from types import MappingProxyType + from typing import List + from lark import Tree, Token + from gdtoolkit.linter.problem import Problem + from gdtoolkit.common.utils import get_line, get_column + + def lint(parse_tree: Tree, config: MappingProxyType) -> List[Problem]: + if "no-print-statement" in config.get("disable", []): + return [] + problems = [] + for call_node in parse_tree.find_data("standalone_call"): + for child in call_node.children: + if isinstance(child, Token) and child.value == "print": + problems.append(Problem( + name="no-print-statement", + description="Avoid using print()", + line=get_line(child), + column=get_column(child), + )) + break + return problems +""") + +CODE_WITH_PRINT = "func foo():\n\tprint('hello')\n" +CODE_WITHOUT_PRINT = "func foo():\n\tpass\n" + + +def test_plugin_loading_with_valid_module(): + code = "func foo():\n pass\n" + config = _config_with_plugin("gdtoolkit.linter.basic_checks") + result = lint_code(code, config) + assert isinstance(result, list) + + +def test_plugin_loading_with_missing_module(): + code = "func foo():\n pass\n" + config = _config_with_plugin("nonexistent_module_xyz") + result = lint_code(code, config) + assert isinstance(result, list) + + +def test_plugin_loading_with_empty_plugins(): + code = "func foo():\n pass\n" + config = DEFAULT_CONFIG.copy() + config.update({"plugins": []}) + result = lint_code(code, config) + assert isinstance(result, list) + + +def test_plugin_loading_with_no_plugins_key(): + code = "func foo():\n pass\n" + result = lint_code(code, DEFAULT_CONFIG) + assert isinstance(result, list) + + +def test_local_plugin_detects_print(tmp_path): + plugin_file = tmp_path / "no_print_checks.py" + plugin_file.write_text(PLUGIN_SOURCE) + + old_cwd = os.getcwd() + os.chdir(tmp_path) + try: + config = _config_with_plugin("no_print_checks") + result = lint_code(CODE_WITH_PRINT, config) + print_problems = [p for p in result if p.name == "no-print-statement"] + assert len(print_problems) == 1 + assert print_problems[0].line == 2 + finally: + os.chdir(old_cwd) + + +def test_local_plugin_no_false_positive(tmp_path): + plugin_file = tmp_path / "no_print_checks.py" + plugin_file.write_text(PLUGIN_SOURCE) + + old_cwd = os.getcwd() + os.chdir(tmp_path) + try: + config = _config_with_plugin("no_print_checks") + result = lint_code(CODE_WITHOUT_PRINT, config) + print_problems = [p for p in result if p.name == "no-print-statement"] + assert len(print_problems) == 0 + finally: + os.chdir(old_cwd) + + +def test_path_based_plugin_loading(tmp_path): + subdir = tmp_path / "ci" / "gdlint" + subdir.mkdir(parents=True) + plugin_file = subdir / "no_print_checks.py" + plugin_file.write_text(PLUGIN_SOURCE) + + old_cwd = os.getcwd() + os.chdir(tmp_path) + try: + config = _config_with_plugin("ci/gdlint/no_print_checks") + result = lint_code(CODE_WITH_PRINT, config) + print_problems = [p for p in result if p.name == "no-print-statement"] + assert len(print_problems) == 1 + assert print_problems[0].line == 2 + finally: + os.chdir(old_cwd) + + +def test_path_based_plugin_missing_file(tmp_path): + old_cwd = os.getcwd() + os.chdir(tmp_path) + try: + config = _config_with_plugin("ci/gdlint/nonexistent") + result = lint_code(CODE_WITH_PRINT, config) + assert isinstance(result, list) + finally: + os.chdir(old_cwd)