Merge pull request #45 from olehermanse/rebase

Refactored linting code and fixed issues

Author: olehermanse

.gitignore

@@ -11,6 +11,7 @@ __pycache__
 /tmp/
 *.log
 *.output.cf
+*.output.txt
 git_diff_exists
 commit_message.txt
 black_output.txt

HACKING.md

@@ -4,6 +4,53 @@ This document aims to have relevant information for people contributing to and m
 It is not necessary for users of the tool to know about these processes.
 For general user information, see the [README](./README.md).
 
+## Code formatting
+
+We use automated code formatters to ensure consistent code style / indentation.
+Please format Python code with [black](https://pypi.org/project/black/), and YAML and markdown files with [Prettier](https://prettier.io/).
+For simplicity's sake, we don't have a custom configuration, we use the tool's defaults.
+
+If your editor does not do this automatically, you can run these tools from the command line:
+
+```bash
+make format
+```
+
+## Installing from source:
+
+For developers working on CFEngine CLI, it is recommended to install an editable version of the tool:
+
+```bash
+make install
+```
+
+Some of the tests require that you have the CLI installed (they run `cfengine` commands).
+
+## Running commands without installing
+
+You can also run commands without installing, using `uv`:
+
+```bash
+uv run cfengine format
+```
+
+## Running tests
+
+Use the makefile command to run all linting and tests:
+
+```bash
+make check
+```
+
+Running individual test suites:
+
+```bash
+uv run pytest
+bash tests/run-lint-tests.sh
+bash tests/run-format-tests.sh
+bash tests/run-shell-tests.sh
+```
+
 ## Releasing new versions
 
 Releases are [automated using a GH Action](https://github.com/cfengine/cfengine-cli/blob/main/.github/workflows/pypi-publish.yml)
@@ -79,62 +126,11 @@ Copy the token and paste it into the GitHub Secret named `PYPI_PASSWORD`.
 `PYPI_USERNAME` should be there already, you don't have to edit it, it is simply `__token__`.
 Don't store the token anywhere else - we generate new tokens if necessary.
 
-## Code formatting
-
-We use automated code formatters to ensure consistent code style / indentation.
-Please format Python code with [black](https://pypi.org/project/black/), and YAML and markdown files with [Prettier](https://prettier.io/).
-For simplicity's sake, we don't have a custom configuration, we use the tool's defaults.
-
-If your editor does not do this automatically, you can run these tools from the command line:
-
-```bash
-black . && prettier . --write
-```
-
-## Running commands during development
-
-This project uses `uv`.
-This makes it easy to run commands without installing the project, for example:
-
-```bash
-uv run cfengine format
-```
-
-## Installing from source:
-
-```bash
-git fetch --all --tags
-pip3 install .
-```
-
-## Running tests
-
-Unit tests:
-
-```bash
-py.test
-```
-
-Shell tests (requires installing first):
-
-```bash
-cat tests/shell/*.sh | bash
-```
-
 ## Not implemented yet / TODOs
 
 - `cfengine run`
   - The command could automatically detect that you have CFEngine installed on a remote hub, and run it there instead (using `cf-remote`).
   - Handle when `cf-agent` is not installed, help users install.
   - Prompt / help users do what they meant (i.e. build and deploy and run).
-- `cfengine format`
-  - Automatically break up and indent method calls, function calls, and nested function calls.
-  - Smarter placement of comments based on context.
-  - The command should be able to take a filename as an argument, and also operate using stdin and stdout.
-    (Receive file content on stdin, file type using command line arg, output formatted file to stdout).
-  - We can add a shortcut, `cfengine fmt`, since that matches other tools, like `deno`.
-- `cfengine lint`
-  - The command should be able to take a filename as an argument, and also take file content from stdin.
-  - It would be nice if we refactored `validate_config()` in `cfbs` so it would take a simple dictionary (JSON) instead of a special CFBSConfig object.
 - Missing commands:
   - `cfengine install` - Install CFEngine packages / binaries (Wrapping `cf-remote install`).

Makefile

@@ -0,0 +1,22 @@
+.PHONY: default format lint install check
+
+default: check
+
+format:
+	uv tool run black .
+	prettier . --write
+
+lint:
+	uv tool run black --check .
+	uv tool run flake8 src/ --ignore=E203,W503,E722,E731 --max-complexity=100 --max-line-length=160
+	uv tool run pyflakes src/
+	uv tool run pyright src/
+
+install:
+	pipx install --force --editable .
+
+check: format lint install
+	uv run pytest
+	bash tests/run-lint-tests.sh
+	bash tests/run-format-tests.sh
+	bash tests/run-shell-tests.sh

README.md

@@ -60,8 +60,8 @@ When it finds a mistake, it points out where the problem is like this;
       ifvarclass => "cfengine";
       ^--------^
 Deprecation: Use 'if' instead of 'ifvarclass' at main.cf:5:7
-FAIL: main.cf (1 errors)
-Failure, 1 errors in total.
+FAIL: main.cf (1 error)
+Failure, 1 error in total.
 ```
 
 Note that since we use a different parser than `cf-agent` / `cf-promises`, they are not 100% in sync.

src/cfengine_cli/commands.py

@@ -4,7 +4,7 @@
 import json
 from cfengine_cli.profile import profile_cfengine, generate_callstack
 from cfengine_cli.dev import dispatch_dev_subcommand
-from cfengine_cli.lint import lint_folder, lint_single_arg
+from cfengine_cli.lint import lint_args
 from cfengine_cli.shell import user_command
 from cfengine_cli.paths import bin
 from cfengine_cli.version import cfengine_cli_version_string
@@ -96,20 +96,16 @@ def format(names, line_length) -> int:
 
 def _lint(files, strict) -> int:
     if not files:
-        return lint_folder(".", strict)
-
-    errors = 0
-
-    for file in files:
-        errors += lint_single_arg(file, strict)
-
-    return errors
+        return lint_args(["."], strict)
+    return lint_args(files, strict)
 
 
 def lint(files, strict) -> int:
     errors = _lint(files, strict)
     if errors == 0:
         print("Success, no errors found.")
+    elif errors == 1:
+        print("Failure, 1 error in total.")
     else:
         print(f"Failure, {errors} errors in total.")
     return errors

src/cfengine_cli/docs.py

@@ -16,7 +16,8 @@
 from cfbs.pretty import pretty_file
 from cfbs.utils import find
 
-from cfengine_cli.lint import lint_folder, lint_policy_file
+from cfengine_cli.format import format_policy_file
+from cfengine_cli.lint import lint_args, lint_policy_file_snippet
 from cfengine_cli.utils import UserError
 
 IGNORED_DIRS = [".git"]
@@ -127,7 +128,7 @@ def fn_check_syntax(
     first_line,
     _last_line,
     snippet_number,
-    prefix=None,
+    prefix,
 ):
     snippet_abs_path = os.path.abspath(snippet_path)
 
@@ -138,8 +139,13 @@ def fn_check_syntax(
 
     match language:
         case "cf":
-            r = lint_policy_file(
-                snippet_abs_path, origin_path, first_line + 1, snippet_number, prefix
+            r = lint_policy_file_snippet(
+                snippet_abs_path,
+                origin_path,
+                first_line + 1,
+                snippet_number,
+                prefix,
+                strict=False,
             )
             if r != 0:
                 raise UserError(f"Error when checking '{origin_path}'")
@@ -189,6 +195,7 @@ def fn_replace(origin_path, snippet_path, _language, first_line, last_line, inde
 
 
 def fn_autoformat(_origin_path, snippet_path, language, _first_line, _last_line):
+    assert language in ("cf", "json")
     match language:
         case "json":
             try:
@@ -203,6 +210,9 @@ def fn_autoformat(_origin_path, snippet_path, language, _first_line, _last_line)
                 raise UserError(f"Couldn't open '{snippet_path}'")
             except json.decoder.JSONDecodeError:
                 raise UserError(f"Invalid json in '{snippet_path}'")
+        case "cf":
+            # Note: Dead code - Not used for CFEngine policy yet
+            format_policy_file(snippet_path, 80)
 
 
 def _translate_language(x):
@@ -241,10 +251,23 @@ def _process_markdown_code_blocks(
     origin_paths = sorted(parsed_markdowns["files"].keys())
     origin_paths_len = len(origin_paths)
 
+    if origin_paths_len == 0:
+        print("No markdown files found.")
+        return
+
+    if syntax_check:
+        # We currently only print the filenames during linting, not formatting
+        print(
+            f"Processing code blocks (snippets) inside {origin_paths_len} markdown files:"
+        )
     for origin_paths_i, origin_path in enumerate(origin_paths):
         percentage = int(100 * (origin_paths_i + 1) / origin_paths_len)
-        prefix = f"[{origin_paths_i + 1}/{origin_paths_len} ({percentage}%)] "
+        spaces = " " * (4 - len(str(percentage)))
+        prefix = f"[{origin_paths_i + 1}/{origin_paths_len}{spaces}({percentage}%)] "
         offset = 0
+        if syntax_check and not parsed_markdowns["files"][origin_path]["code-blocks"]:
+            print(f"{prefix}SKIP: No code blocks in '{origin_path}'")
+            continue
         for i, code_block in enumerate(
             parsed_markdowns["files"][origin_path]["code-blocks"]
         ):
@@ -259,33 +282,44 @@ def _process_markdown_code_blocks(
             snippet_path = f"{origin_path}.snippet-{snippet_number}.{language}"
 
             flags = code_block["flags"]
+            first_line = code_block["first_line"]
+            last_line = code_block["last_line"]
             if "noextract" in flags or "skip" in flags:
                 # code block was marked to be skipped
+                if syntax_check:
+                    print(
+                        f"{prefix}SKIP: Snippet {snippet_number} at '{origin_path}:{first_line}' ({language} {' '.join(flags)})"
+                    )
                 continue
             if extract:
                 fn_extract(
                     origin_path,
                     snippet_path,
                     language,
-                    code_block["first_line"],
-                    code_block["last_line"],
+                    first_line,
+                    last_line,
                 )
 
-            if syntax_check and "novalidate" not in flags:
-                try:
-                    fn_check_syntax(
-                        origin_path,
-                        snippet_path,
-                        language,
-                        code_block["first_line"],
-                        code_block["last_line"],
-                        snippet_number,
-                        prefix,
+            if syntax_check:
+                if "novalidate" in flags:
+                    print(
+                        f"{prefix}SKIP: Snippet {snippet_number} at '{origin_path}:{first_line}' ({language} {' '.join(flags)})"
                     )
-                except Exception as e:
-                    if cleanup:
-                        os.remove(snippet_path)
-                    raise e
+                else:
+                    try:
+                        fn_check_syntax(
+                            origin_path,
+                            snippet_path,
+                            language,
+                            first_line,
+                            last_line,
+                            snippet_number,
+                            prefix,
+                        )
+                    except Exception as e:
+                        if cleanup:
+                            os.remove(snippet_path)
+                        raise e
 
             if output_check and "noexecute" not in flags:
                 fn_check_output()
@@ -295,16 +329,16 @@ def _process_markdown_code_blocks(
                     origin_path,
                     snippet_path,
                     language,
-                    code_block["first_line"],
-                    code_block["last_line"],
+                    first_line,
+                    last_line,
                 )
 
             if replace and "noreplace" not in flags:
                 offset = fn_replace(
                     origin_path,
                     snippet_path,
                     language,
-                    code_block["first_line"],
+                    first_line,
                     code_block["last_line"],
                     code_block["indent"],
                 )
@@ -409,7 +443,7 @@ def check_docs() -> int:
 
     Run by the command:
     cfengine dev lint-docs"""
-    r = lint_folder(".", strict=False)
+    r = lint_args(["."], strict=False)
     if r != 0:
         return r
     _process_markdown_code_blocks(