diff --git a/scripts/snapshots/report_cel_detailed.md b/scripts/snapshots/report_cel_detailed.md index d507dd96..bd303a88 100644 --- a/scripts/snapshots/report_cel_detailed.md +++ b/scripts/snapshots/report_cel_detailed.md @@ -1,6 +1,6 @@ # cloudformation-validate vs cfn-lint - Parity Report -> Generated: 2026-10-16 17:44:00 +> Generated: 2026-10-20 10:55:28 > Engine: **cel** > Detail level: **detailed** > Matching: `(rule_id, resource_id, path)` two-pass with `(rule_id, resource_id)` fallback + aliases @@ -23,14 +23,14 @@ | Metric | Value | |--------|------:| | True Positives | 5177 | -| False Positives (engine bugs) | 321 | +| False Positives (engine bugs) | 320 | | Engine Extra (correct, cfn-lint gap) | 8897 | | False Negatives (engine misses) | 404 | -| Precision | 94.16% | +| Precision | 94.18% | | Recall | 92.76% | | F1 | 93.46% | -| Unique rules detected | 255 | -| Perfect templates | 446/732 | +| Unique rules detected | 254 | +| Perfect templates | 447/732 | | Location mismatches (matched pairs) | 9 | ### By Severity @@ -38,7 +38,7 @@ | Severity | TP | FP | EE | FN | Precision | Recall | |----------|---:|---:|---:|---:|----------:|-------:| | Fatal | 477 | 15 | 81 | 140 | 96.95% | 77.31% | -| Error | 924 | 62 | 15 | 157 | 93.71% | 85.48% | +| Error | 924 | 61 | 15 | 157 | 93.81% | 85.48% | | Warning | 2160 | 12 | 388 | 65 | 99.45% | 97.08% | | Info | 1616 | 232 | 8413 | 42 | 87.45% | 97.47% | @@ -1154,7 +1154,7 @@ but found another document - **W6001** → `Outputs.ImportedValue.Value.Fn::ImportValue` L39 in `good_output_value_string_yaml` > The output value {'Fn::ImportValue': 'SomeExportedName'} is an import from another output -## False Positives - 321 extra findings across 27 rules +## False Positives - 320 extra findings across 26 rules These are diagnostics the engine reports but cfn-lint does not expect (potential bugs). @@ -1831,11 +1831,6 @@ These are diagnostics the engine reports but cfn-lint does not expect (potential - **W3689** `Cloned` (AWS::RDS::DBCluster) → `Properties.StorageEncrypted` L26 in `bad_W3688_snapshot_identifier_from_parameter_yaml` > 'StorageEncrypted' is ignored when SourceDBClusterIdentifier is present -### E1155 - 1 extra - -- **E1155** `InvalidLiteralName` (AWS::Logs::LogGroup) → `Properties.LogGroupName` L8 in `bad_F3031_log_group_name_dollar_brace_yaml` - > 'invalid ${literal' does not match format 'AWS::Logs::LogGroup.Name' - ### E3001 - 1 extra - Basic CloudFormation Resource Check - **E3001** `myBucketFirstAndLastPass` (AWS::S3::Bucket) L19 in `bad_core_directives_yaml` @@ -19814,7 +19809,7 @@ These are correct diagnostics the engine reports that cfn-lint does not cover. - **W9054** `CertAuth` (AWS::ACMPCA::CertificateAuthorityActivation) → `Properties.Certificate` L8 in `bad_schema_write_only_yaml` > Write-only property 'Certificate' of 'CertAuth' is referenced in output 'WriteOnlyOutput' -## Per-Template Breakdown - 286 templates with mismatches +## Per-Template Breakdown - 285 templates with mismatches ### `good_lifecycle_intrinsic_scenarios_yaml` - 16 mismatches (2 TP, 0 FP, 0 EE, 16 FN) @@ -20986,11 +20981,6 @@ These are correct diagnostics the engine reports that cfn-lint does not cover. - FN: `F3006` - EE: `W9013` ×2, `I9001`, `I9040` -### `bad_F3031_log_group_name_dollar_brace_yaml` - 1 mismatches (1 TP, 1 FP, 2 EE, 0 FN) - -- FP: `E1155` -- EE: `I9001`, `I9040` - ### `bad_F3032_whole_properties_item_cardinality_yaml` - 1 mismatches (1 TP, 1 FP, 1 EE, 0 FN) - FP: `F3032` @@ -21289,11 +21279,11 @@ other tool's output. They are excluded from precision/recall scoring. | Cause | Count | % of FP | Rules | |-------|------:|--------:|-------| -| Extra informational findings | 220 | 68.54% | I4010 | -| Other | 49 | 15.26% | E0001, F0018, F2002, F3012, F3016, F3017, F3032 | -| Over-reporting property/intrinsic errors | 28 | 8.72% | E1155, E3001, E3019, E3022, E3023, E3024, E3029, E3048, E3055, E3505, E3510 | -| Stricter than cfn-lint (informational) | 12 | 3.74% | I3011 | -| Stricter than cfn-lint (warnings) | 12 | 3.74% | W1028, W1030, W2001, W2530, W3688, W3689, W4011 | +| Extra informational findings | 220 | 68.75% | I4010 | +| Other | 49 | 15.31% | E0001, F0018, F2002, F3012, F3016, F3017, F3032 | +| Over-reporting property/intrinsic errors | 27 | 8.44% | E3001, E3019, E3022, E3023, E3024, E3029, E3048, E3055, E3505, E3510 | +| Stricter than cfn-lint (informational) | 12 | 3.75% | I3011 | +| Stricter than cfn-lint (warnings) | 12 | 3.75% | W1028, W1030, W2001, W2530, W3688, W3689, W4011 | ## Location Mismatches - 9 matched pairs disagree on line diff --git a/scripts/snapshots/report_composite_detailed.md b/scripts/snapshots/report_composite_detailed.md index 5359ae58..6a674349 100644 --- a/scripts/snapshots/report_composite_detailed.md +++ b/scripts/snapshots/report_composite_detailed.md @@ -1,6 +1,6 @@ # cloudformation-validate vs cfn-lint - Parity Report -> Generated: 2026-10-16 17:44:05 +> Generated: 2026-10-20 10:55:31 > Engine: **composite** > Detail level: **detailed** > Matching: `(rule_id, resource_id, path)` two-pass with `(rule_id, resource_id)` fallback + aliases @@ -23,14 +23,14 @@ | Metric | Value | |--------|------:| | True Positives | 5177 | -| False Positives (engine bugs) | 321 | +| False Positives (engine bugs) | 320 | | Engine Extra (correct, cfn-lint gap) | 8897 | | False Negatives (engine misses) | 404 | -| Precision | 94.16% | +| Precision | 94.18% | | Recall | 92.76% | | F1 | 93.46% | -| Unique rules detected | 255 | -| Perfect templates | 446/732 | +| Unique rules detected | 254 | +| Perfect templates | 447/732 | | Location mismatches (matched pairs) | 9 | ### By Severity @@ -38,7 +38,7 @@ | Severity | TP | FP | EE | FN | Precision | Recall | |----------|---:|---:|---:|---:|----------:|-------:| | Fatal | 477 | 15 | 81 | 140 | 96.95% | 77.31% | -| Error | 924 | 62 | 15 | 157 | 93.71% | 85.48% | +| Error | 924 | 61 | 15 | 157 | 93.81% | 85.48% | | Warning | 2160 | 12 | 388 | 65 | 99.45% | 97.08% | | Info | 1616 | 232 | 8413 | 42 | 87.45% | 97.47% | @@ -1154,7 +1154,7 @@ but found another document - **W6001** → `Outputs.ImportedValue.Value.Fn::ImportValue` L39 in `good_output_value_string_yaml` > The output value {'Fn::ImportValue': 'SomeExportedName'} is an import from another output -## False Positives - 321 extra findings across 27 rules +## False Positives - 320 extra findings across 26 rules These are diagnostics the engine reports but cfn-lint does not expect (potential bugs). @@ -1831,11 +1831,6 @@ These are diagnostics the engine reports but cfn-lint does not expect (potential - **W3689** `Cloned` (AWS::RDS::DBCluster) → `Properties.StorageEncrypted` L26 in `bad_W3688_snapshot_identifier_from_parameter_yaml` > 'StorageEncrypted' is ignored when SourceDBClusterIdentifier is present -### E1155 - 1 extra - -- **E1155** `InvalidLiteralName` (AWS::Logs::LogGroup) → `Properties.LogGroupName` L8 in `bad_F3031_log_group_name_dollar_brace_yaml` - > 'invalid ${literal' does not match format 'AWS::Logs::LogGroup.Name' - ### E3001 - 1 extra - Basic CloudFormation Resource Check - **E3001** `myBucketFirstAndLastPass` (AWS::S3::Bucket) L19 in `bad_core_directives_yaml` @@ -19814,7 +19809,7 @@ These are correct diagnostics the engine reports that cfn-lint does not cover. - **W9054** `CertAuth` (AWS::ACMPCA::CertificateAuthorityActivation) → `Properties.Certificate` L8 in `bad_schema_write_only_yaml` > Write-only property 'Certificate' of 'CertAuth' is referenced in output 'WriteOnlyOutput' -## Per-Template Breakdown - 286 templates with mismatches +## Per-Template Breakdown - 285 templates with mismatches ### `good_lifecycle_intrinsic_scenarios_yaml` - 16 mismatches (2 TP, 0 FP, 0 EE, 16 FN) @@ -20986,11 +20981,6 @@ These are correct diagnostics the engine reports that cfn-lint does not cover. - FN: `F3006` - EE: `W9013` ×2, `I9001`, `I9040` -### `bad_F3031_log_group_name_dollar_brace_yaml` - 1 mismatches (1 TP, 1 FP, 2 EE, 0 FN) - -- FP: `E1155` -- EE: `I9001`, `I9040` - ### `bad_F3032_whole_properties_item_cardinality_yaml` - 1 mismatches (1 TP, 1 FP, 1 EE, 0 FN) - FP: `F3032` @@ -21289,11 +21279,11 @@ other tool's output. They are excluded from precision/recall scoring. | Cause | Count | % of FP | Rules | |-------|------:|--------:|-------| -| Extra informational findings | 220 | 68.54% | I4010 | -| Other | 49 | 15.26% | E0001, F0018, F2002, F3012, F3016, F3017, F3032 | -| Over-reporting property/intrinsic errors | 28 | 8.72% | E1155, E3001, E3019, E3022, E3023, E3024, E3029, E3048, E3055, E3505, E3510 | -| Stricter than cfn-lint (informational) | 12 | 3.74% | I3011 | -| Stricter than cfn-lint (warnings) | 12 | 3.74% | W1028, W1030, W2001, W2530, W3688, W3689, W4011 | +| Extra informational findings | 220 | 68.75% | I4010 | +| Other | 49 | 15.31% | E0001, F0018, F2002, F3012, F3016, F3017, F3032 | +| Over-reporting property/intrinsic errors | 27 | 8.44% | E3001, E3019, E3022, E3023, E3024, E3029, E3048, E3055, E3505, E3510 | +| Stricter than cfn-lint (informational) | 12 | 3.75% | I3011 | +| Stricter than cfn-lint (warnings) | 12 | 3.75% | W1028, W1030, W2001, W2530, W3688, W3689, W4011 | ## Location Mismatches - 9 matched pairs disagree on line diff --git a/scripts/snapshots/report_rego_detailed.md b/scripts/snapshots/report_rego_detailed.md index 81d19d13..3fac00fe 100644 --- a/scripts/snapshots/report_rego_detailed.md +++ b/scripts/snapshots/report_rego_detailed.md @@ -1,6 +1,6 @@ # cloudformation-validate vs cfn-lint - Parity Report -> Generated: 2026-10-16 17:43:55 +> Generated: 2026-10-20 10:55:25 > Engine: **rego** > Detail level: **detailed** > Matching: `(rule_id, resource_id, path)` two-pass with `(rule_id, resource_id)` fallback + aliases @@ -23,14 +23,14 @@ | Metric | Value | |--------|------:| | True Positives | 5177 | -| False Positives (engine bugs) | 321 | +| False Positives (engine bugs) | 320 | | Engine Extra (correct, cfn-lint gap) | 8897 | | False Negatives (engine misses) | 404 | -| Precision | 94.16% | +| Precision | 94.18% | | Recall | 92.76% | | F1 | 93.46% | -| Unique rules detected | 255 | -| Perfect templates | 446/732 | +| Unique rules detected | 254 | +| Perfect templates | 447/732 | | Location mismatches (matched pairs) | 9 | ### By Severity @@ -38,7 +38,7 @@ | Severity | TP | FP | EE | FN | Precision | Recall | |----------|---:|---:|---:|---:|----------:|-------:| | Fatal | 477 | 15 | 81 | 140 | 96.95% | 77.31% | -| Error | 924 | 62 | 15 | 157 | 93.71% | 85.48% | +| Error | 924 | 61 | 15 | 157 | 93.81% | 85.48% | | Warning | 2160 | 12 | 388 | 65 | 99.45% | 97.08% | | Info | 1616 | 232 | 8413 | 42 | 87.45% | 97.47% | @@ -1154,7 +1154,7 @@ but found another document - **W6001** → `Outputs.ImportedValue.Value.Fn::ImportValue` L39 in `good_output_value_string_yaml` > The output value {'Fn::ImportValue': 'SomeExportedName'} is an import from another output -## False Positives - 321 extra findings across 27 rules +## False Positives - 320 extra findings across 26 rules These are diagnostics the engine reports but cfn-lint does not expect (potential bugs). @@ -1831,11 +1831,6 @@ These are diagnostics the engine reports but cfn-lint does not expect (potential - **W3689** `Cloned` (AWS::RDS::DBCluster) → `Properties.StorageEncrypted` L26 in `bad_W3688_snapshot_identifier_from_parameter_yaml` > 'StorageEncrypted' is ignored when SourceDBClusterIdentifier is present -### E1155 - 1 extra - -- **E1155** `InvalidLiteralName` (AWS::Logs::LogGroup) → `Properties.LogGroupName` L8 in `bad_F3031_log_group_name_dollar_brace_yaml` - > 'invalid ${literal' does not match format 'AWS::Logs::LogGroup.Name' - ### E3001 - 1 extra - Basic CloudFormation Resource Check - **E3001** `myBucketFirstAndLastPass` (AWS::S3::Bucket) L19 in `bad_core_directives_yaml` @@ -19814,7 +19809,7 @@ These are correct diagnostics the engine reports that cfn-lint does not cover. - **W9054** `CertAuth` (AWS::ACMPCA::CertificateAuthorityActivation) → `Properties.Certificate` L8 in `bad_schema_write_only_yaml` > Write-only property 'Certificate' of 'CertAuth' is referenced in output 'WriteOnlyOutput' -## Per-Template Breakdown - 286 templates with mismatches +## Per-Template Breakdown - 285 templates with mismatches ### `good_lifecycle_intrinsic_scenarios_yaml` - 16 mismatches (2 TP, 0 FP, 0 EE, 16 FN) @@ -20986,11 +20981,6 @@ These are correct diagnostics the engine reports that cfn-lint does not cover. - FN: `F3006` - EE: `W9013` ×2, `I9001`, `I9040` -### `bad_F3031_log_group_name_dollar_brace_yaml` - 1 mismatches (1 TP, 1 FP, 2 EE, 0 FN) - -- FP: `E1155` -- EE: `I9001`, `I9040` - ### `bad_F3032_whole_properties_item_cardinality_yaml` - 1 mismatches (1 TP, 1 FP, 1 EE, 0 FN) - FP: `F3032` @@ -21289,11 +21279,11 @@ other tool's output. They are excluded from precision/recall scoring. | Cause | Count | % of FP | Rules | |-------|------:|--------:|-------| -| Extra informational findings | 220 | 68.54% | I4010 | -| Other | 49 | 15.26% | E0001, F0018, F2002, F3012, F3016, F3017, F3032 | -| Over-reporting property/intrinsic errors | 28 | 8.72% | E1155, E3001, E3019, E3022, E3023, E3024, E3029, E3048, E3055, E3505, E3510 | -| Stricter than cfn-lint (informational) | 12 | 3.74% | I3011 | -| Stricter than cfn-lint (warnings) | 12 | 3.74% | W1028, W1030, W2001, W2530, W3688, W3689, W4011 | +| Extra informational findings | 220 | 68.75% | I4010 | +| Other | 49 | 15.31% | E0001, F0018, F2002, F3012, F3016, F3017, F3032 | +| Over-reporting property/intrinsic errors | 27 | 8.44% | E3001, E3019, E3022, E3023, E3024, E3029, E3048, E3055, E3505, E3510 | +| Stricter than cfn-lint (informational) | 12 | 3.75% | I3011 | +| Stricter than cfn-lint (warnings) | 12 | 3.75% | W1028, W1030, W2001, W2530, W3688, W3689, W4011 | ## Location Mismatches - 9 matched pairs disagree on line diff --git a/src/Cargo.lock b/src/Cargo.lock index 94c38df6..e834963b 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -476,6 +476,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" + [[package]] name = "cfn-validate" version = "1.14.0" @@ -1365,6 +1371,18 @@ dependencies = [ "byteorder", ] +[[package]] +name = "nix" +version = "0.31.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf20d2fde8ff38632c426f1165ed7436270b44f199fc55284c38276f9db47c3d" +dependencies = [ + "bitflags 2.13.1", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "nom" version = "7.1.3" @@ -2754,14 +2772,15 @@ checksum = "e13c156562582aa81c60cb29407084cdb54c4164760106ab78e6c5b0858cf64e" [[package]] name = "zip" -version = "8.6.0" +version = "9.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d04a6b5381502aa6087c94c669499eb1602eb9c5e8198e534de571f7154809b" +checksum = "6e04976da629d3b04204ba8105b396a193af8ad2583b2c79bfb3ab8af580a11e" dependencies = [ "crc32fast", "flate2", "indexmap 2.14.0", "memchr", + "nix", "typed-path", "zopfli", ] diff --git a/src/data-source/Cargo.toml b/src/data-source/Cargo.toml index 9586edde..e3dd0b33 100644 --- a/src/data-source/Cargo.toml +++ b/src/data-source/Cargo.toml @@ -43,7 +43,7 @@ ruzstd = { version = "0.9.1", default-features = false, features = ["std"] } zstd = { version = "0.13", default-features = false, optional = true } lz4_flex = { version = "0.13", default-features = false, features = ["std"], optional = true } ureq = { version = "3.2.0", optional = true } -zip = { version = "8.2.0", default-features = false, features = ["deflate"], optional = true } +zip = { version = "9.0.5", default-features = false, features = ["deflate"], optional = true } log = { version = "0.4", optional = true } env_logger = { version = "0.11", optional = true } anyhow = { version = "1", optional = true } diff --git a/src/data-source/src/schema.rs b/src/data-source/src/schema.rs index 8fbab352..b9828375 100644 --- a/src/data-source/src/schema.rs +++ b/src/data-source/src/schema.rs @@ -90,8 +90,12 @@ pub fn download_schemas(upstream_dir: &Path) -> anyhow::Result<(SyncStats, Strin // type→hash set the resource schemas are written from. let mut region_files: Vec = archive .file_names() - .filter(|n| n.starts_with("providers/") && n.ends_with(".json") && *n != "providers/sam.json") - .map(String::from) + .map(|name| { + name.context("enhanced schema archive contains an invalid entry name").map(|name| name.into_owned()) + }) + .collect::>>()? + .into_iter() + .filter(|n| n.starts_with("providers/") && n.ends_with(".json") && n != "providers/sam.json") .collect(); // Read us-east-1 first so its hash wins for any type present in several regions. region_files.sort_by_key(|n| (!n.ends_with("/us-east-1.json"), n.clone())); diff --git a/src/resources/expected/validation_reports1.json b/src/resources/expected/validation_reports1.json index e3bf02c3..19918e8d 100644 --- a/src/resources/expected/validation_reports1.json +++ b/src/resources/expected/validation_reports1.json @@ -8734,7 +8734,7 @@ "resourcesScanned": 1, "counts": { "fatal": 1, - "errors": 1, + "errors": 0, "warnings": 0, "informational": 2, "debug": 0 @@ -8768,29 +8768,6 @@ "expectedConstraint": "^[.\\-_/#A-Za-z0-9]{1,512}\\Z" } }, - { - "ruleId": "E1155", - "severity": "ERROR", - "message": "'invalid ${literal' does not match format 'AWS::Logs::LogGroup.Name'", - "source": "CFN_LINT", - "entity": { - "logicalId": "InvalidLiteralName", - "entityType": "Resource", - "resourceType": "AWS::Logs::LogGroup" - }, - "propertyPath": "Properties.LogGroupName", - "category": "Intrinsic Function", - "startLine": 8, - "startColumn": 7, - "endLine": 8, - "endColumn": 19, - "documentationUrl": "https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-logs.git", - "ruleDescription": "Validate CloudWatch logs group name", - "phase": "SCHEMA", - "context": { - "actualValue": "invalid ${literal" - } - }, { "ruleId": "I9001", "severity": "INFO", @@ -8960,9 +8937,9 @@ "propertyPath": "Properties.Value", "category": "Schema", "startLine": 28, - "startColumn": 7, + "startColumn": 22, "endLine": 28, - "endColumn": 12, + "endColumn": 32, "ruleDescription": "Property type mismatch", "phase": "SCHEMA", "context": { diff --git a/src/schema-validator/src/validate.rs b/src/schema-validator/src/validate.rs index 509921cf..c1d74371 100644 --- a/src/schema-validator/src/validate.rs +++ b/src/schema-validator/src/validate.rs @@ -3486,6 +3486,7 @@ fn validate_prop( } } + let mut schema_pattern_failures = Vec::new(); if let Some(ref pat) = schema.pattern && let Some(re) = compile_pattern(pat) { @@ -3517,13 +3518,14 @@ fn validate_prop( None, condition_map(conds), )); + schema_pattern_failures.push((val.clone(), conds.clone())); } } } } if let Some(ref fmt) = schema.format { - validate_format(out, m, rid, prop_path, fmt, &scenarios); + validate_format(out, m, rid, prop_path, fmt, &scenarios, &schema_pattern_failures); } for (val, conds) in &scenarios { @@ -4825,7 +4827,7 @@ fn validate_reference_type( } _ => return, }; - out.push(build_diagnostic( + let mut diagnostic = build_diagnostic( "F3012", &format!( "{} ({}) returns '{}', but property expects '{}'", @@ -4838,7 +4840,11 @@ fn validate_reference_type( rid, prop_path, None, - )); + ); + if let Some(intrinsic_name) = m.authored_intrinsic_name_at(rid, prop_path) { + diagnostic.location = Some(m.resource_span(rid, &format!("{prop_path}.{intrinsic_name}"))); + } + out.push(diagnostic); } } ResolvedValue::TypedDynamic { reason: _name, param_type } => { @@ -5143,6 +5149,7 @@ fn validate_format( prop_path: &str, format: &str, scenarios: &[(serde_json::Value, HashMap)], + schema_pattern_failures: &[(serde_json::Value, HashMap)], ) { let Some(re) = FORMAT_PATTERNS.get(format) else { return; @@ -5162,6 +5169,13 @@ fn validate_format( } if !re.is_match(&s) { let rule_id = format_rule_for_format(format).unwrap_or("E1103"); + if rule_id == "E1155" + && schema_pattern_failures + .iter() + .any(|(pattern_value, pattern_conditions)| pattern_value == val && pattern_conditions == conds) + { + continue; + } out.push(build_diagnostic_conditional( rule_id, &format!("{} does not match format '{}'", format_value(val), format), diff --git a/src/schema-validator/tests/integration.rs b/src/schema-validator/tests/integration.rs index 1f52472c..2197b497 100644 --- a/src/schema-validator/tests/integration.rs +++ b/src/schema-validator/tests/integration.rs @@ -473,14 +473,12 @@ fn property_f3031_pattern_violation() { } #[test] -fn literal_dollar_brace_log_group_name_enforces_pattern_and_format() { +fn literal_dollar_brace_log_group_name_reports_schema_pattern_once() { let diagnostics = validate_fixture("bad/F3031_log_group_name_dollar_brace.yaml"); - let relevant: Vec<_> = - diagnostics.iter().filter(|diagnostic| matches!(diagnostic.rule_id.as_str(), "F3031" | "E1155")).collect(); + let pattern_diagnostics = diags_for(&diagnostics, "F3031"); - assert_eq!(relevant.len(), 2, "the literal must violate both schema constraints: {diagnostics:?}"); - assert!(relevant.iter().any(|diagnostic| diagnostic.rule_id == "F3031")); - assert!(relevant.iter().any(|diagnostic| diagnostic.rule_id == "E1155")); + assert_eq!(pattern_diagnostics.len(), 1, "expected one schema pattern diagnostic: {diagnostics:?}"); + assert!(!has_rule(&diagnostics, "E1155"), "format validation must not duplicate F3031: {diagnostics:?}"); } #[test] @@ -493,6 +491,19 @@ fn unresolved_log_group_name_substitution_is_not_schema_validated_as_a_literal() ); } +#[test] +fn getatt_type_mismatch_anchors_at_authored_intrinsic() { + let diagnostics = validate_fixture("bad/F6101_getatt_ref_typed_object_attribute.yaml"); + let diagnostic = diagnostics + .iter() + .find(|diagnostic| diagnostic.rule_id == "F3012" && diagnostic.resource_logical_id() == Some("Parameter")) + .expect("expected the GetAtt object-to-string type mismatch"); + + assert_eq!(diagnostic.property_path.as_deref(), Some("Properties.Value")); + let location = diagnostic.location.expect("type mismatch must have an authored source location"); + assert_eq!((location.start_line, location.start_column), (28, 22)); +} + #[test] fn read_only_property_not_flagged() { // The read-only-property check was removed: it never fires on real templates diff --git a/src/template-model/src/parser/yaml.rs b/src/template-model/src/parser/yaml.rs index a50e9673..624becaa 100644 --- a/src/template-model/src/parser/yaml.rs +++ b/src/template-model/src/parser/yaml.rs @@ -298,10 +298,10 @@ struct CfnYamlLoader { /// `MAX_YAML_ALIAS_EXPANSION_NODES`. alias_expanded_nodes: usize, /// Stack of tags to apply when sequences/mappings complete. - /// Each entry is (tag_name, stack_depth_when_set). + /// Each entry is (tag name, stack depth when set, tagged value position). /// A stack is needed because nested tags (e.g. `!Or [!Equals [...]]`) /// would otherwise overwrite the outer tag. - pending_tags: Vec<(String, usize)>, + pending_tags: Vec<(String, usize, (u32, u32))>, /// One frame per open container, parallel to `doc_stack`; see [`PathFrame`]. path_frames: Vec, span_map: HashMap, @@ -515,14 +515,17 @@ impl CfnYamlLoader { .unwrap_or_else(|| format!("{}{}", FN_PREFIX, tag_name)) } - fn record_tagged_spans(&mut self, tag_name: &str) { + fn record_tagged_spans(&mut self, tag_name: &str, tagged_value_position: (u32, u32)) { let base_path = self.current_path(); if base_path.is_empty() { return; } - let tagged_path = format!("{}/{}", base_path, Self::cfn_tag_key(tag_name)); - if let Some(span) = self.span_map.get(&base_path).copied() { + let function_name = Self::cfn_tag_key(tag_name); + let tagged_path = format!("{base_path}/{function_name}"); + if function_name == FN_GET_ATT { + self.span_map.insert(tagged_path.clone(), tagged_value_position); + } else if let Some(span) = self.span_map.get(&base_path).copied() { self.span_map.entry(tagged_path.clone()).or_insert(span); } @@ -547,16 +550,16 @@ impl CfnYamlLoader { fn insert_new_node(&mut self, node: (Yaml, usize), mark: Marker) { let (mut node_val, aid) = node; - if let Some((_, depth)) = self.pending_tags.last() + if let Some((_, depth, _)) = self.pending_tags.last() && self.doc_stack.len() == *depth { - let (tag_name, _) = self.pending_tags.pop().unwrap(); + let (tag_name, _, tagged_value_position) = self.pending_tags.pop().unwrap(); // A `!Name` shorthand tag is unambiguously a function invocation - // unlike a long-form `Fn::Name` map key, which may be a data key - // so any unknown tag warrants the unknown-function warning here, // where the tag context still exists. self.warn_unknown_tag(&tag_name); - self.record_tagged_spans(&tag_name); + self.record_tagged_spans(&tag_name, tagged_value_position); let wrapped = Self::wrap_with_tag(&tag_name, node_val); node_val = wrapped; } @@ -789,7 +792,7 @@ impl MarkedEventReceiver for CfnYamlLoader { return; } if let Some(tag_name) = Self::cfn_tag_name(tag) { - self.pending_tags.push((tag_name, self.doc_stack.len())); + self.pending_tags.push((tag_name, self.doc_stack.len(), Self::mark_position(mark))); } // Anchor the sequence at its slot in the parent before opening a frame // for its elements. As a mapping value this is a no-op (the key already @@ -810,7 +813,7 @@ impl MarkedEventReceiver for CfnYamlLoader { return; } if let Some(tag_name) = Self::cfn_tag_name(tag) { - self.pending_tags.push((tag_name, self.doc_stack.len())); + self.pending_tags.push((tag_name, self.doc_stack.len(), Self::mark_position(mark))); } self.record_value_span(mark); self.doc_stack.push((Yaml::Hash(Hash::new()), aid)); @@ -902,7 +905,7 @@ impl MarkedEventReceiver for CfnYamlLoader { if let Some(tag_name) = cfn_tag { self.warn_unknown_tag(&tag_name); - self.record_tagged_spans(&tag_name); + self.record_tagged_spans(&tag_name, Self::mark_position(mark)); let wrapped = Self::wrap_with_tag(&tag_name, node); self.insert_new_node((wrapped, aid), mark); } else { @@ -1279,6 +1282,11 @@ mod tests { } o => panic!("Expected GetAtt, got {:?}", o), } + let span = ir + .span_index + .get("Resources/R/Properties/Role/Fn::GetAtt") + .expect("shorthand GetAtt must retain its tag location"); + assert_eq!((span.start_line, span.start_column), (5, 21)); } #[test]