Run commands for phony targets

[leynos]

Summary

• ensure phony targets invoke their associated actions
• clarify intermediate representation comment for phony outputs
• test that phony targets run their commands

Testing

• make fmt
• make lint
• make test

---

https://chatgpt.com/codex/tasks/task_e_6894eec97ed8832294ac2e3f0bb468b7

Summary by Sourcery

Ensure phony targets invoke their commands in generated ninja files and verify this with comprehensive parametrized integration tests. Clarify IR representation for phony outputs and unify edge formatting.

New Features:

• Add parametrized ninja integration tests covering script execution and phony target commands

Enhancements:

• Output action_id for phony edges instead of default 'phony' rule in ninja generator
• Clarify BuildEdge.phony doc comment to denote non-file outputs
• Update feature tests to assert phony commands appear in generated ninja manifests

Tests:

• Introduce AssertionType enum and ninja_integration_setup fixture for streamlined integration testing
• Consolidate multiple scenarios into a single parametrized rstest function

[coderabbitai]

Summary by CodeRabbit

• Documentation

  • Clarified the description of phony targets in user-facing documentation.

• Bug Fixes

  • Improved handling of phony targets so that they now correctly run their associated commands, rather than being treated as a special case.

• Tests

  • Updated and consolidated integration tests for Ninja build files, ensuring phony targets are properly tested for command execution.
  • Enhanced test coverage and maintainability with parameterised test cases.

Walkthrough

Update the documentation for the phony field in BuildEdge to clarify its meaning. Remove special handling of "phony" in Ninja rule name generation, always using the action ID instead. Adjust related tests to reflect this change and add an integration test to verify that a phony target executes its command.

Changes

Cohort / File(s)	Change Summary
Documentation Update src/ir.rs	Update the phony field doc comment in BuildEdge to clarify that it means the output does not correspond to a real file.
Ninja Generation Logic src/ninja_gen.rs	Remove the conditional branch for phony edges; always print the edge's action_id as the rule name.
Feature Test Update tests/features/ninja.feature	Rename scenario and update assertions to check for command execution, not the literal "phony" rule.
Unit & Integration Tests tests/ninja_gen_tests.rs	Rename test, update expected manifest, and add a new integration test for command execution of a phony target.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test Runner
    participant BuildGraph as Build Graph
    participant NinjaGen as Ninja Generator
    participant Ninja as Ninja Tool
    participant Shell as Shell

    Test->>BuildGraph: Create phony target with shell command
    Test->>NinjaGen: Generate Ninja manifest
    NinjaGen->>Test: Write manifest to temp dir
    Test->>Ninja: Run Ninja build for phony target
    Ninja->>Shell: Execute shell command (e.g., touch file)
    Shell-->>Ninja: Command completes
    Ninja-->>Test: Build complete
    Test->>Test: Assert output file exists

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Add intermediate representation structures #32: Introduces the BuildEdge struct and related IR structures, directly related to the documentation update for phony.
• Document phony and always targets #4: Implements and documents the phony and always flags, closely connected to the changes in phony handling and Ninja generation.

Poem

Phony no more, the rule's now clear,
Action IDs ring out—let all engineers cheer!
With tests that run and touch a file,
The build is honest, free of guile.
Ninja commands now plainly shown,
In manifest lines, the truth is known.
🛠️✨

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c558bd2 and af0980a.

📒 Files selected for processing (1)

• tests/ninja_gen_tests.rs (4 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

⚙️ CodeRabbit Configuration File

**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.

• Adhere to single responsibility and CQRS

• Place function attributes after doc comments.

• Do not use return in single-line functions.

• Move conditionals with >2 branches into a predicate function.

• Avoid unsafe unless absolutely necessary.

• Every module must begin with a //! doc comment that explains the module's purpose and utility.

• Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar

• Lints must not be silenced except as a last resort.

  • #[allow] is forbidden.
  • Only narrowly scoped #[expect(lint, reason = "...")] is allowed.
  • No lint groups, no blanket or file-wide suppression.
  • Include FIXME: with link if a fix is expected.

• Use rstest fixtures for shared setup and to avoid repetition between tests.

• Replace duplicated tests with #[rstest(...)] parameterised cases.

• Prefer mockall for mocks/stubs.

• Prefer .expect() over .unwrap()

• Ensure that any API or behavioural changes are reflected in the documentation in docs/

• Ensure that any completed roadmap steps are recorded in the appropriate roadmap in docs/

• Files must not exceed 400 lines in length

  • Large modules must be decomposed
  • Long match statements or dispatch tables should be decomposed by domain and collocated with targets
  • Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.

Files:

• tests/ninja_gen_tests.rs

🔍 MCP Research (1 server)

Deepwiki:

• The IR generation validates targets for duplicate outputs and rule existence, raising errors such as IrGenError::DuplicateOutput and IrGenError::RuleNotFound accordingly. The phony flag affects build graph validation and Ninja file generation, ensuring phony targets run their commands even if outputs exist. (src/ir.rs)

• The Ninja file generation module removed the special-case printing of the literal "phony" rule name, now always printing the action ID as the rule name, simplifying the Ninja file output for phony targets. (src/ninja_gen.rs)

• The test suite refactored multiple integration tests into a single parameterized test function, verifying that the Ninja manifest no longer contains the literal "phony" keyword but includes the command to run for phony targets. The feature test scenario was renamed to "Phony target runs its command" to reflect this behavior. (tests/ninja_gen_tests.rs, tests/features/ninja.feature)

• The documentation comment for the phony field in BuildEdge was clarified to state "Output does not correspond to a real file," emphasizing that phony targets represent non-file outputs that always run their commands. (src/ir.rs)

🔇 Additional comments (5)

tests/ninja_gen_tests.rs (5)

12-12: LGTM: Import addition follows guidelines.

The fixture import from rstest aligns with the coding guideline to use rstest fixtures for shared setup.

---

14-14: LGTM: Import addition supports fixture implementation.

The TempDir import is necessary for the new fixture pattern.

---

30-36: LGTM: Well-designed enum with clear variants.

The AssertionType enum provides a clean abstraction for different test assertion modes. The variants are self-explanatory and support the unified test approach.

---

38-46: LGTM: Fixture correctly implements shared setup pattern.

The fixture properly handles Ninja availability checking and provides Option<TempDir> to enable test skipping when Ninja is unavailable. This follows the rstest guidelines for shared setup.

---

49-73: Verify test case name alignment with behaviour.

The test case name phony_target_runs_command correctly reflects the behaviour change where phony targets now execute their commands. The expected output shows the action ID a instead of the literal "phony" keyword, which aligns with the PR objectives.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/fix-test-failures-in-code

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai generate unit tests to generate unit tests for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[sourcery-ai]

Reviewer's Guide

This PR refactors the Ninja generator to emit real action rules for phony targets (instead of a special 'phony' rule), updates the IR phony flag comment to clarify that outputs aren’t real files, and expands both unit and integration tests to validate that phony and script commands actually run.

Sequence diagram for emitting action rules for phony targets

sequenceDiagram
    participant NinjaGen as Ninja Generator
    participant BuildEdge
    participant NinjaFile

    NinjaGen->>BuildEdge: Check if edge.phony
    BuildEdge-->>NinjaGen: Return phony flag
    NinjaGen->>NinjaFile: Emit action rule using edge.action_id (even if phony)
    Note right of NinjaFile: No special 'phony' rule, always emit action

Loading

Class diagram for BuildEdge and DisplayEdge changes

classDiagram
    class BuildEdge {
        +Vec<PathBuf> implicit_outputs
        +Vec<PathBuf> order_only_deps
        +bool phony "Output does not correspond to a real file."
        +bool always
    }
    class DisplayEdge {
        +Display for DisplayEdge
        +edge: &BuildEdge
    }
    DisplayEdge --> BuildEdge : uses

Loading

File-Level Changes

Change	Details	Files
Always emit the action rule for phony build edges	Removed special-case 'phony' rule selection in DisplayEdge Wrote action_id directly in build lines regardless of phony flag	src/ninja_gen.rs
Clarify phony flag semantics in IR	Updated IR comment to state phony outputs aren’t real files	src/ir.rs
Overhaul integration tests with param-driven fixtures	Introduced AssertionType enum and ninja_integration_setup fixture Consolidated multiple integration tests into one rstest with cases Added a phony target case to verify command execution	tests/ninja_gen_tests.rs
Update feature scenario for phony command checks	Renamed scenario to 'Phony target runs its command' Assert presence of 'build clean:' and the 'rm -rf build' command	tests/features/ninja.feature

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[codescene-delta-analysis]

Gates Failed
Enforce advisory code health rules (1 file with Code Duplication)

Gates Passed
5 Quality Gates Passed

See analysis details in CodeScene

Reason for failure

Enforce advisory code health rules	Violations	Code Health Impact
ninja_gen_tests.rs	1 advisory rule	10.00 → 9.39	Suppress

Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

[sourcery-ai]

Hey @leynos - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[leynos]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[leynos]

@sourcery-ai review

[sourcery-ai]

Hey @leynos - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}