diff --git a/.gitignore b/.gitignore index d5e4f15..5ad7957 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ -**/__pycache__ \ No newline at end of file +**/__pycache__ +venv +.venv \ No newline at end of file diff --git a/LintingCheck.py b/LintingCheck.py new file mode 100644 index 0000000..2fb8596 --- /dev/null +++ b/LintingCheck.py @@ -0,0 +1,121 @@ +from argparse import ArgumentParser +from pathlib import Path +from sys import stderr, exit +from typing import Optional +from pathspec import PathSpec +from config import load_mypy_arguments, load_ignore_patterns +from utils.file_utils import should_ignore_file +from mypy.api import run as mypy_api_run + + +def main() -> int: + """Main entry point for the style checker. + + Returns: + Exit code (0 for success, 1 for errors found) + """ + print("Starting the Linting Check") + + parser = ArgumentParser(description='Modern Python style checker') + parser.add_argument('paths', nargs='*', default=['.'], help='Paths to check (default: current directory)') + + args = parser.parse_args() + + ignore_patterns = load_ignore_patterns() + mypy_args = load_mypy_arguments() + + all_errors: list = [] + + for path_str in args.paths: + path = Path(path_str) + if path.is_file(): + if should_ignore_file(path, ignore_patterns): + continue + else: + mypy_return = run_mypy_on_file(path_str, mypy_args) + all_errors.extend(mypy_return) + elif path.is_dir(): + mypy_return = run_mypy_on_directory(path, ignore_patterns, mypy_args) + all_errors.extend(mypy_return) + else: + print(f"Warning: Path not found: {path}", file=stderr) + + if all_errors: + for mypy_error in all_errors: + print(mypy_error) + print(f"\nFound {len(all_errors)} MyPy errors.") + return 1 + + else: + print("Passed all MyPy checks!") + return 0 + + +def run_mypy_on_directory(directory: Path, ignore_patterns: Optional[PathSpec], args: list[str] | None = None) -> list: + """Check all Python files in a directory recursively. + + Args: + directory: Directory to check + ignore_patterns: Patterns for files to ignore + args: A list of all of the arguments to be passed into MyPy + + Returns: + list of style errors found + """ + errors_in_directory: list = [] + + for file_path in directory.rglob('*.py'): + if should_ignore_file(file_path, ignore_patterns): + continue + + else: + mypy_errors = run_mypy_on_file(str(file_path), args) + errors_in_directory.extend(mypy_errors) + + return errors_in_directory + + +def run_mypy_on_file(file_path_string: str, args: list[str] | None = None) -> list: + """Run mypy on a single python file. + + Args: + file_path_string: The string representation of the path to the + file that should be checked + args: The arguments that should be passed into the mypy call + + Returns: + The result of calling mypy on the file + """ + args_with_file_path_at_start = [] + + if not args: + args_with_file_path_at_start = [f"{file_path_string}", "--strict"] + else: + args_with_file_path_at_start.insert(0, f"{file_path_string}") + + mypy_output_to_standard, mypy_output_to_error, mypy_return_value = mypy_api_run(args_with_file_path_at_start) + + # Note that despite what mypy says, it actually writes the various linting errors to the *standard* + # output, not the error output. It writes fatal errors caused by odds and ends to it's error output, + # so it's necessary to check both the standard the the error output to actually find all of the + # desired errors + + # If mypy returns with no errors, we can return an empty list of errors + if mypy_return_value == 0: + return [] + + # Otherwise we need to filter everything mypy prints so that we only have the return errors + errors_to_return = [] + for output_source in (mypy_output_to_standard, mypy_output_to_error): + for line in output_source.splitlines(): + if "error:" in line: + errors_to_return.append(line) + + return errors_to_return + + + + + +if __name__ == '__main__': + exit(main()) \ No newline at end of file diff --git a/README.md b/README.md index 9d2b02a..3d56fec 100644 --- a/README.md +++ b/README.md @@ -50,27 +50,53 @@ The above entries in a `.standardignore` would have the checker skip over the fo Using the `.standardignore` file specified in the section above, specific function and variable names can be skipped over on the check. -The syntax to do so is the use of the `!` before the name of the variable/function to ignore. +The syntax to do so is the use of the `name:` before the name of the variable/function to ignore. For example ```cmd -!sleep_for_retry +name: sleep_for_retry ``` The above example ignores the sleep_for_retry function when applying standards as the name is required as it is an overwrite of an outside modules functionality. ## Running locally so you don't have to wait on github actions -from the root fo the repo you're trying to check, run the following in a terminal: +From the root of the repo you're trying to check, run the following in a terminal: ```cmd python "path-to-this-repo/StandardCheck.py" ``` -So, if I were trying to run it on TreeTapper, and both TreeTapper and PythonStandardAction were in the same folder, I would run +The `python` command uses whatever virtual environment is currently active. Before running the checker, install this action's requirements into that environment: ```cmd +pip install -r "path-to-this-repo/requirements.txt" +``` + +So, if I were trying to run it on TreeTapper, and both TreeTapper and PythonStandardAction were in the same folder, I would activate TreeTapper's virtual environment and run: + +```cmd +pip install -r "../PythonStandardAction/requirements.txt" python "../PythonStandardAction/StandardCheck.py" ``` +This process can also be done for the linting step by running the same commands, but replacing the `StandardCheck.py` reference with the `LintingCheck.py` reference as follows: + +```cmd +python "../PythonStandardAction/LintingCheck.py" +``` + +If the repository has an in-repo virtual environment, add it to `.standardignore` so `StandardCheck.py` and `LintingCheck.py` do not scan installed packages while walking the repository. + + +## Notes about the MyPy checker step + +The MyPy checker step is going to default to running with the '--strict' call on every file in the application unless the entire file is ignored. Note that it doesn't ignore specific functions, classes, or lines since they are instead ignored with the usual "# type: ignore" comment that it otherwise uses. If you want to run this with different MyPy settings and arguments, simply add "mypyargs: [Your arguments here]" to the ".standardignore" file in the format as follows: + +```cmd +mypyargs: --ignore-missing-imports --deprecated-calls-exclude +``` + +Note that doing something like this will automatically disable the '--strict' tag in MyPy unless it is specifically included in the arguments. Regardless of if custom args are passed in or not, this step will automatically figure out the needed file path arguments so it is not necessary to add this to the beginning and will actually cause errors if you do. + diff --git a/StandardCheck.py b/StandardCheck.py index 7bf9cfe..718fd33 100644 --- a/StandardCheck.py +++ b/StandardCheck.py @@ -16,7 +16,7 @@ import checkers.complexity as complexity_module -def visit_node(node: ast.AST, file_path: str, ignore_codes: set[str], ignore_names: set[str] = None) -> list[models.StyleError]: +def visit_node(node: ast.AST, file_path: str, ignore_codes: set[str], ignore_names: set[str] | None = None) -> list[models.StyleError]: """Visit an AST node and perform checks. Args: @@ -28,7 +28,8 @@ def visit_node(node: ast.AST, file_path: str, ignore_codes: set[str], ignore_nam Returns: list of style errors found """ - ignore_names = ignore_names or set() + if not ignore_names: + ignore_names = set() if isinstance(node, ast.ClassDef): return common_nodes_module.check_class(node, file_path, ignore_codes, ignore_names) @@ -40,7 +41,7 @@ def visit_node(node: ast.AST, file_path: str, ignore_codes: set[str], ignore_nam return [] -def check_file(file_path: Path, ignore_codes: set[str], ignore_names: set[str] = None, config: dict[str, Any] = None) -> list[models.StyleError]: +def check_file(file_path: Path, ignore_codes: set[str], ignore_names: set[str] | None = None, config: dict[str, Any] | None = None) -> list[models.StyleError]: """Check a single Python file. Args: @@ -52,9 +53,13 @@ def check_file(file_path: Path, ignore_codes: set[str], ignore_names: set[str] = Returns: list of style errors found """ + if not ignore_names: + ignore_names = set() + + if not config: + config = {} + errors = [] - ignore_names = ignore_names or set() - config = config or {} max_complexity = config.get('max_complexity', 15) max_indentation = config.get('max_indentation', 4) diff --git a/action.yml b/action.yml index a641e86..ca7c6f6 100644 --- a/action.yml +++ b/action.yml @@ -14,7 +14,15 @@ runs: - name: Install dependencies from requirements.txt shell: bash run: pip install -r "${{ github.action_path }}/requirements.txt" + + - name: Get any missing type stubs for MyPy check + shell: bash + run: mypy --install-types --non-interactive - - name: Run Check + - name: Run Standard Check + shell: bash + run: python "${{ github.action_path }}/StandardCheck.py" + + - name: Run mypy check with args shell: bash - run: python "${{ github.action_path }}/StandardCheck.py" \ No newline at end of file + run: \ No newline at end of file diff --git a/checkers/common_nodes.py b/checkers/common_nodes.py index 4416386..df5c5d1 100644 --- a/checkers/common_nodes.py +++ b/checkers/common_nodes.py @@ -6,7 +6,7 @@ import utils.patterns as patterns_module import checkers.error_creation as error_creation_module -def check_variable(node: ast.Name, file_path: str, ignore_codes: set[str], ignore_names: set[str] = None) -> list[models.StyleError]: +def check_variable(node: ast.Name, file_path: str, ignore_codes: set[str], ignore_names: set[str] = set()) -> list[models.StyleError]: """Check variable naming. Args: @@ -18,8 +18,7 @@ def check_variable(node: ast.Name, file_path: str, ignore_codes: set[str], ignor Returns: list of style errors found """ - errors = [] - ignore_names = ignore_names or set() + errors: list[models.StyleError] = [] # Skip if name should be ignored if file_utils_module.should_ignore_name(node.id, ignore_names): @@ -38,7 +37,7 @@ def check_variable(node: ast.Name, file_path: str, ignore_codes: set[str], ignor return errors -def check_class(node: ast.ClassDef, file_path: str, ignore_codes: set[str], ignore_names: set[str] = None) -> list[models.StyleError]: +def check_class(node: ast.ClassDef, file_path: str, ignore_codes: set[str], ignore_names: set[str] = set()) -> list[models.StyleError]: """Check class definition. Args: @@ -50,8 +49,7 @@ def check_class(node: ast.ClassDef, file_path: str, ignore_codes: set[str], igno Returns: list of style errors found """ - errors = [] - ignore_names = ignore_names or set() + errors: list[models.StyleError] = [] # Skip if name should be ignored if file_utils_module.should_ignore_name(node.name, ignore_names): @@ -98,7 +96,7 @@ def _is_valid_class_name(name: str) -> bool: return False -def check_function(node: ast.FunctionDef | ast.AsyncFunctionDef, file_path: str, ignore_codes: set[str], ignore_names: set[str] = None) -> list[models.StyleError]: +def check_function(node: ast.FunctionDef | ast.AsyncFunctionDef, file_path: str, ignore_codes: set[str], ignore_names: set[str] = set()) -> list[models.StyleError]: """Check function definition. Args: @@ -110,8 +108,7 @@ def check_function(node: ast.FunctionDef | ast.AsyncFunctionDef, file_path: str, Returns: list of style errors found """ - errors = [] - ignore_names = ignore_names or set() + errors: list[models.StyleError] = [] is_test_file = 'test' in file_path.lower() if file_utils_module.should_ignore_name(node.name, ignore_names): @@ -199,7 +196,7 @@ def _check_function_docstrings(node: ast.FunctionDef | ast.AsyncFunctionDef, fil Returns: list of style errors related to function docstrings """ - errors = [] + errors: list[models.StyleError] = [] # Skip docstring checks for @overload functions if _has_overload_decorator(node): @@ -230,7 +227,7 @@ def _check_docstring_format(node: ast.FunctionDef | ast.AsyncFunctionDef | ast.C Returns: list of style errors found """ - errors = [] + errors: list[models.StyleError] = [] if not docstring: return errors summary = docstring.split('\n\n')[0].strip() @@ -264,7 +261,7 @@ def _check_function_docstring(node: ast.FunctionDef | ast.AsyncFunctionDef, file Returns: list of style errors found """ - errors = [] + errors: list[models.StyleError] = [] docstring = ast.get_docstring(node) if not docstring: diff --git a/checkers/complexity.py b/checkers/complexity.py index 0c79ffe..3bede76 100644 --- a/checkers/complexity.py +++ b/checkers/complexity.py @@ -3,7 +3,7 @@ import models as models -def check_complexity(tree: ast.Module, content: str, file_path: str, ignore_codes: set[str], max_complexity: int = 15, max_indentation: int = 4) -> list[models.StyleError]: +def check_complexity(tree: ast.AST, content: str, file_path: str, ignore_codes: set[str], max_complexity: int = 15, max_indentation: int = 4) -> list[models.StyleError]: """Check cyclomatic complexity and indentation depth for all functions in a file. Args: diff --git a/checkers/imports.py b/checkers/imports.py index a235921..ca9345e 100644 --- a/checkers/imports.py +++ b/checkers/imports.py @@ -24,7 +24,7 @@ def collect_imports(tree: ast.AST) -> tuple[list[ast.Import | ast.ImportFrom], l return imports + import_froms, import_froms -def check_imports(all_imports: list[ast.AST], import_froms: list[ast.ImportFrom], used_names: set[str], file_path: Path, ignore_codes: set[str]) -> list[models.StyleError]: +def check_imports(all_imports: list[ast.Import | ast.ImportFrom], import_froms: list[ast.ImportFrom], used_names: set[str], file_path: Path, ignore_codes: set[str]) -> list[models.StyleError]: """Run all import-related checks. Args: @@ -56,7 +56,7 @@ def _check_import_order(imports: list[ast.Import | ast.ImportFrom], file_path: s Returns: list of style errors found """ - errors = [] + errors: list[models.StyleError] = [] if not imports: return errors @@ -164,7 +164,7 @@ def _check_unused_imports(imports: list[ast.Import | ast.ImportFrom], names_used try: with open(file_path, 'r', encoding='utf-8') as f: file_content = f.read() - except: + except(Exception): file_content = "" for imp in imports: @@ -217,7 +217,7 @@ def _check_unused_from_import_nodes(imp: ast.ImportFrom, names_used: set[str], f Returns: an unused import error, or None """ - errors = [] + errors: list[models.StyleError] = [] # Never flag __future__ imports as unused if imp.module == '__future__': diff --git a/checkers/security.py b/checkers/security.py index 11990ff..49fc940 100644 --- a/checkers/security.py +++ b/checkers/security.py @@ -115,7 +115,7 @@ def _check_sql_injection_call(node: ast.AST, file_path: str, ignore_codes: set[s Returns: the sql injection errors, if any """ - errors = [] + errors: list[models.StyleError] = [] if not isinstance(node, ast.Call): return errors if not (isinstance(node.func, ast.Attribute) and node.func.attr in ['execute', 'executemany']): @@ -167,7 +167,7 @@ def _check_shell_injection_call(node: ast.AST, file_path: str, ignore_codes: set Returns: the shell injection errors, if any """ - errors = [] + errors: list[models.StyleError] = [] if not isinstance(node, ast.Call): return errors is_bad_attr = isinstance(node.func, ast.Attribute) and node.func.attr in ['system', 'popen', 'spawn', 'exec'] diff --git a/config.py b/config.py index d1a8dcc..ef50ea6 100644 --- a/config.py +++ b/config.py @@ -42,13 +42,41 @@ def load_ignore_patterns() -> Optional[pathspec.PathSpec]: return pathspec.PathSpec.from_lines('gitwildmatch', patterns) +def load_mypy_arguments() -> list[str] | None: + """This function loads all of the aruments needed to override the default + ones for MyPy, if applicable, from the .standardignore file. + + Returns: + A list of arguments to pass into MyPy, or None + """ + + argument_file = Path('.standardignore') + args = [] + + if not argument_file.exists(): + return None + + with open(argument_file, 'r', encoding='utf-8') as f: + lines = f.readlines() + + for line in lines: + line = line.strip() + if line.startswith("mypyargs:"): + args = line[9:].strip().split() + + if args: + return args + else: + return None + + def load_ignore_names() -> set[str]: """Load specific names to ignore from .standardignore file. Returns: Set of names to ignore in style checking """ - ignore_names = set() + ignore_names: set = set() ignore_file = Path('.standardignore') if not ignore_file.exists(): diff --git a/requirements.txt b/requirements.txt index 1855cd0..1b89842 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,2 @@ -pathspec==0.12.1 \ No newline at end of file +pathspec ~= 1.1 +mypy ~= 2.1 \ No newline at end of file