Skip to content

Serialise Recipe for manifest output #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
leynos wants to merge 2 commits into from
+23 −6
Closed

Serialise Recipe for manifest output #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07ea4dd
Add Recipe serialiser
leynos Aug 5, 2025
575a36c
Derive Recipe serialisation via untagged enum
leynos Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ pub struct Rule {
/// Exactly one variant must be provided for a rule or target. The fields are
/// flattened in the manifest, so the presence of `command`, `script`, or `rule`
/// determines the variant.
#[derive(Debug, Clone, PartialEq)]
#[derive(Debug, Clone, PartialEq, Serialize)]
#[serde(untagged)]
pub enum Recipe {
/// A single shell command.
Command { command: String },
Expand Down
8 changes: 4 additions & 4 deletions src/ir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,10 @@
states: &mut HashMap<PathBuf, VisitState>,
) -> Option<Vec<PathBuf>> {
for dep in deps {
if targets.contains_key(dep)
&& let Some(cycle) = visit(targets, dep, stack, states)
{
return Some(cycle);
if targets.contains_key(dep) {

Check failure on line 344 in src/ir.rs

View workflow job for this annotation

GitHub Actions / build-test 

this `if` statement can be collapsed
if let Some(cycle) = visit(targets, dep, stack, states) {
return Some(cycle);
}
Comment on lines +344 to +347
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the collapsible if statement flagged by Clippy.

The nested if statement violates the coding guidelines requirement that Clippy warnings must be disallowed. The pipeline failure correctly identifies this can be collapsed.

Apply this diff to collapse the nested conditions:

-            if targets.contains_key(dep) {
-                if let Some(cycle) = visit(targets, dep, stack, states) {
-                    return Some(cycle);
-                }
-            }
+            if targets.contains_key(dep) && let Some(cycle) = visit(targets, dep, stack, states) {
+                return Some(cycle);
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if targets.contains_key(dep) {
if let Some(cycle) = visit(targets, dep, stack, states) {
return Some(cycle);
}
if targets.contains_key(dep) && let Some(cycle) = visit(targets, dep, stack, states) {
return Some(cycle);
}
🧰 Tools
🪛 GitHub Check: build-test

[failure] 344-344:
this if statement can be collapsed

🪛 GitHub Actions: CI

[error] 344-344: Clippy lint error: this if statement can be collapsed.

🤖 Prompt for AI Agents 
In src/ir.rs around lines 344 to 347, there is a nested if statement that Clippy
flags for collapsing. To fix this, combine the two conditions into a single if
statement using a logical AND, so that the inner if is merged with the outer
one, eliminating the nested structure and satisfying Clippy's requirement.

}
}
None
Expand Down
18 changes: 17 additions & 1 deletion tests/ast_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! Unit tests for Netsuke manifest AST deserialisation.
//! Unit tests for Netsuke manifest AST serialisation and deserialisation.

use netsuke::{ast::*, manifest};
use rstest::rstest;
use semver::Version;
use serde_yml::Value;

/// Convenience wrapper around the library manifest parser for tests.
fn parse_manifest(yaml: &str) -> anyhow::Result<NetsukeManifest> {
Expand Down Expand Up @@ -362,3 +363,18 @@ fn invalid_manifests_fail(#[case] file: &str) {
let path = format!("tests/data/{file}");
assert!(manifest::from_path(&path).is_err());
}

#[rstest]
#[case(Recipe::Command { command: "echo".into() }, "command: echo")]
#[case(Recipe::Script { script: "run".into() }, "script: run")]
#[case(
Recipe::Rule {
rule: StringOrList::List(vec!["a".into(), "b".into()]),
},
concat!("rule:\n", " - a\n", " - b"),
)]
fn serialize_recipe_variants(#[case] recipe: Recipe, #[case] expected_yaml: &str) {
let actual: Value = serde_yml::to_value(&recipe).expect("serialize");
let expected: Value = serde_yml::from_str(expected_yaml).expect("yaml");
assert_eq!(actual, expected);
}
Loading