[beman-tidy] Implement cmake.default check

[neatudarius]

Implements #41

[Copilot]

Pull request overview

Adds a new Beman Standard CMake rule (cmake.default) to verify that the main library target is created unconditionally in the root CMakeLists.txt, along with test coverage and representative CMake input fixtures.

Changes:

• Implement CMakeDefaultCheck.check() and register it as cmake.default.
• Add pytest coverage for valid/invalid cmake.default scenarios.
• Add new CMakeLists.txt fixture files for valid and invalid patterns.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
beman_tidy/lib/checks/beman_standard/cmake.py	Adds the cmake.default check implementation and a (currently non-fixing) fix() stub.
tests/lib/checks/beman_standard/cmake/test_cmake.py	Adds new tests for cmake.default (valid/invalid) and a skipped fix-inplace test.
tests/lib/checks/beman_standard/cmake/data/valid/valid-default-v1.txt	Adds a valid CMake example where the main library target is unconditional.
tests/lib/checks/beman_standard/cmake/data/invalid/invalid-default-v1.txt	Adds an invalid CMake example where the library target is behind an option defaulting to OFF.
tests/lib/checks/beman_standard/cmake/data/invalid/invalid-default-v2.txt	Adds an invalid CMake example where the library target exists only inside an if().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

beman_tidy/lib/checks/beman_standard/cmake.py:213

• The PR description says it “Implements #41”, but issue #41 explicitly includes implementing the fix-inplace behavior. fix() here is still a stub that only logs and returns False, so --fix-inplace for cmake.default is not implemented and the issue would remain incomplete. Either implement an actual in-place fix (and enable the corresponding test) or adjust the PR description/scope accordingly.

    def fix(self):
        self.log(
            "Please update the root CMakeLists.txt so that the main library target is built unconditionally by default. "
            "See https://github.com/bemanproject/beman/blob/main/docs/beman_standard.md#cmakedefault for more information."
        )
        return False

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

beman_tidy/lib/checks/beman_standard/cmake.py:173

• ast_tree is an iterable returned by cmake_parser.parser.parse_tree(...) and is consumed when iterated. This check() iterates ast_tree in _has_unconditional_library_target() and then iterates it again in _get_option_defaults() / _library_if_conditions(), which can leave those later analyses empty and produce less accurate diagnostics. Materialize the tree once (as other checks in this file do) before reusing it.

        ast_tree = self.get_cmake_parse_tree()

beman_tidy/lib/checks/beman_standard/cmake.py:213

• The PR/issue context for cmake.default calls out implementing both check() (dry-run) and fix() (fix-inplace). This fix() currently only logs guidance and returns False, so running in fix-inplace mode won’t actually remediate anything. Either implement an actual in-place fix for the common patterns being detected (e.g., move add_library(...) out of if()/OFF guards), or update the PR description/issue linkage to reflect that only the dry-run check is implemented in this PR.

    def fix(self):
        self.log(
            "Please update the root CMakeLists.txt so that the main library target is built unconditionally by default. "
            "See https://github.com/bemanproject/beman/blob/main/docs/beman_standard.md#cmakedefault for more information."
        )
        return False

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

beman_tidy/lib/checks/beman_standard/cmake.py:212

• PR metadata says "Implements #41", but issue #41 explicitly includes implementing fix for fix-inplace mode. This fix() implementation currently only logs guidance and returns False, and the corresponding test is marked skipped, so the issue’s fix requirement isn’t actually implemented/validated in this PR.

    def fix(self):
        self.log(
            "Please update the root CMakeLists.txt so that the main library target is built unconditionally by default. "
            "See https://github.com/bemanproject/beman/blob/main/docs/beman_standard.md#cmakedefault for more information."
        )
        return False

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

beman_tidy/lib/checks/beman_standard/cmake.py:159

• _get_option_defaults() currently depends on Option nodes, but option(...) is commonly parsed as a generic Command with identifier == "option". As written, this can leave option_defaults empty and make the OFF-default detection path dead. Consider detecting options via Command.identifier instead (and stripping quotes from arg values).

    def _get_option_defaults(self, ast_tree):
        options = {}
        for item in ast_tree:
            if isinstance(item, Option) and item.args:
                args = [arg.value for arg in item.args]
                if args:
                    options[args[0]] = args[2] if len(args) >= 3 else None
        return options

beman_tidy/lib/checks/beman_standard/cmake.py:143

• CMake keywords are case-insensitive; checking command.args[1].value == "ALIAS" can miss alias, Alias, etc. Make the comparison case-insensitive to avoid false positives/negatives when evaluating add_library forms.

            return False
        if len(command.args) >= 2 and command.args[1].value.upper() == "ALIAS":
            return False

beman_tidy/lib/checks/beman_standard/cmake.py:212

• The PR description says it "Implements #41", but cmake.default still has a stub fix() that always returns False (and the corresponding fix-inplace test is skipped). If #41’s scope includes implementing fix-inplace behavior, either implement fix() for this check or adjust the PR/issue linkage to reflect that only dry-run validation is provided.

    def fix(self):
        self.log(
            "Please update the root CMakeLists.txt so that the main library target is built unconditionally by default. "
            "See https://github.com/bemanproject/beman/blob/main/docs/beman_standard.md#cmakedefault for more information."
        )
        return False

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

beman_tidy/lib/checks/beman_standard/cmake.py:212

• Issue #41 (referenced by this PR) explicitly calls for implementing both check() and fix() for cmake.default. This fix() implementation is still a stub that always returns False, so fix-inplace mode cannot remediate cmake.default failures and the PR doesn’t fully satisfy the linked issue/PR description.

    def fix(self):
        self.log(
            "Please update the root CMakeLists.txt so that the main library target is built unconditionally by default. "
            "See https://github.com/bemanproject/beman/blob/main/docs/beman_standard.md#cmakedefault for more information."
        )
        return False