Create new struct Entity and add logical ids to all diagnostics if available (#206)

Author: satyakigh

scripts/compare_benchmarks.py

@@ -319,13 +319,14 @@ def _per_template_dir(engine, fmt, binding):
 def _diag_sort_key(d):
     """Stable ordering for pairing diagnostics between binding outputs — identity
     that should be binding-invariant (rule id + source span + message)."""
+    entity = d.get("entity") or {}
     return (
         d.get("ruleId") or "",
         d.get("startLine") or 0,
         d.get("startColumn") or 0,
         d.get("endLine") or 0,
         d.get("endColumn") or 0,
-        d.get("resourceId") or "",
+        entity.get("logicalId") or "",
         d.get("propertyPath") or "",
         d.get("message") or "",
     )

scripts/compare_cfnlint.py

@@ -380,9 +380,17 @@ def load_engine_results():
             rule_id = d.get("ruleId", "")
             severity = d.get("severity", "")
             severity = _ENGINE_SEV_MAP.get(severity, severity)
-            resource_id = d.get("resourceId", "")
+            entity = d.get("entity") or {}
+            resource_id = entity.get("logicalId", "") if entity.get("entityType") == "Resource" else ""
+            resource_type = entity.get("resourceType", "")
             resource_path = d.get("propertyPath", "")
-            if resource_path.startswith("Outputs/"):
+            if rule_id == "F0000":
+                # cfn-lint's E0000 parse-error records never carry a Path, so the
+                # engine's richer identity (entity + duplicated-key path) would
+                # defeat both matching passes; compare on the bare rule instead.
+                resource_id = ""
+                resource_path = ""
+            elif resource_path.startswith("Outputs/"):
                 resource_path = resource_path.replace("/", ".")
                 resource_id = ""
             elif resource_id and resource_path.startswith("Outputs."):
@@ -394,7 +402,7 @@ def load_engine_results():
                 "severity": severity,
                 "message": d.get("message", ""),
                 "resource_id": resource_id,
-                "resource_type": d.get("resourceType", ""),
+                "resource_type": resource_type,
                 "resource_path": resource_path,
                 "line": d.get("startLine", 0),
                 "end_line": d.get("endLine", 0),

scripts/snapshots/report_cel_detailed.md

@@ -125,20 +125,27 @@ data class RuleFilterConfig(
     val idRanges: List<IdRange> = emptyList(),                 // numeric ranges, e.g. IdRange("E", 3000, 3099)
     val idPatterns: List<String> = emptyList(),                // regex patterns matched against rule IDs
     val resourceIds: List<ResourceIdFilter> = emptyList(),     // a rule (or every rule) on a logical resource ID
+    val logicalIds: List<LogicalIdFilter> = emptyList(),       // a rule (or every rule) on a named template entity
     val resourceTypes: List<ResourceTypeFilter> = emptyList(), // a rule (or every rule) on a resource type
     val services: List<ServiceFilter> = emptyList(),           // a rule (or every rule) on a service, e.g. "AWS::AutoScaling"
 )
 
-// resourceIds / resourceTypes / services each carry a nullable ruleId:
+// resourceIds / logicalIds / resourceTypes / services each carry a nullable ruleId:
 // set it to scope the filter to one rule, or leave it null for every rule on the target.
 data class ResourceIdFilter(val ruleId: String? = null, val resourceId: String)
+data class LogicalIdFilter(val ruleId: String? = null, val logicalId: String, val entityType: EntityType? = null)
 data class ResourceTypeFilter(val ruleId: String? = null, val resourceType: String)
 data class ServiceFilter(val ruleId: String? = null, val service: String)
 ```
 
 The `service` is matched verbatim against the `service-provider::service-name` prefix of the resource type — its first
 two `::`-delimited segments (e.g. `AWS::AutoScaling` in `AWS::AutoScaling::LaunchConfiguration`).
 
+The `resourceIds` dimension matches only diagnostics attributed to a resource; `logicalIds` additionally matches
+diagnostics on parameters, outputs, mappings, conditions, and template rules (for resource diagnostics the two carry
+the same value). A non-null `entityType` scopes a `LogicalIdFilter` to entities of one type, so `MyThing` as a
+`PARAMETER` is matched without touching a same-named entity of another type.
+
 ### PseudoParameterOverrides
 
 Override CloudFormation pseudo-parameters used during intrinsic function resolution. All fields optional — when `null`,
@@ -209,7 +216,7 @@ data class StandardReport(
 ```
 
 `DetailedReport` has the same structure but its diagnostics include additional fields: `documentationUrl`,
-`ruleDescription`, `phase` (`PARSE` | `SCHEMA` | `LINT`), `section`, and `context` (`ViolationContext` with
+`ruleDescription`, `phase` (`PARSE` | `SCHEMA` | `LINT`), and `context` (`ViolationContext` with
 `actualValue`, `expectedConstraint`, `resolutionSource`, etc.).
 
 ### StandardDiagnostic
@@ -220,9 +227,8 @@ data class StandardDiagnostic(
     val severity: Severity,                // FATAL, ERROR, WARN, INFO, DEBUG
     val message: String,
     val source: RuleOrigin,                // SCHEMA, CFN_LINT, ENGINE, CUSTOM, GUARD
-    val resourceId: String?,               // logical resource ID
-    val resourceType: String?,             // e.g. "AWS::S3::Bucket"
-    val propertyPath: String?,             // e.g. "Properties/BucketName"
+    val entity: Entity?,                   // the named template entity the finding targets, if any
+    val propertyPath: String?,             // e.g. "Properties.BucketName", or section-absolute like "Parameters/MyParam/Type"
     val suggestedFix: String?,
     val category: String?,
     val startLine: UInt?,
@@ -232,4 +238,17 @@ data class StandardDiagnostic(
     val relatedResources: List<RelatedResource>?,
     val conditionScenario: Map<String, Boolean>?,  // condition truth assignment that triggers this
 )
+
+// The named template entity a diagnostic is attributed to. The entity type is the
+// singular form of the top-level template section the entity is declared in.
+data class Entity(
+    val logicalId: String,                 // logical ID as declared in the template
+    val entityType: EntityType,
+    val resourceType: String? = null,      // CloudFormation type, when the entity is a resource whose type is known
+)
+
+enum class EntityType {
+    RESOURCE, PARAMETER, OUTPUT, MAPPING, METADATA,
+    RULE, CONDITION, TRANSFORM, FORMAT_VERSION, DESCRIPTION,
+}
 ```

scripts/snapshots/report_rego_detailed.md

@@ -105,20 +105,27 @@ interface RuleFilterConfig {
     idRanges?: IdRange[];              // numeric ranges, e.g. { prefix: "E", start: 3000, end: 3099 }
     idPatterns?: string[];             // regex patterns matched against rule IDs
     resourceIds?: ResourceIdFilter[];  // a rule (or every rule) on a logical resource ID
+    logicalIds?: LogicalIdFilter[];    // a rule (or every rule) on a named template entity
     resourceTypes?: ResourceTypeFilter[]; // a rule (or every rule) on a resource type
     services?: ServiceFilter[];        // a rule (or every rule) on a service, e.g. "AWS::AutoScaling"
 }
 
-// resourceIds / resourceTypes / services each carry an optional ruleId:
+// resourceIds / logicalIds / resourceTypes / services each carry an optional ruleId:
 // set it to scope the filter to one rule, or omit it for every rule on the target.
 interface ResourceIdFilter   { ruleId?: string; resourceId: string; }
+interface LogicalIdFilter    { ruleId?: string; logicalId: string; entityType?: EntityType; }
 interface ResourceTypeFilter { ruleId?: string; resourceType: string; }
 interface ServiceFilter      { ruleId?: string; service: string; }
 ```
 
 The `service` is matched verbatim against the `service-provider::service-name` prefix of the resource type — its first
 two `::`-delimited segments (e.g. `AWS::AutoScaling` in `AWS::AutoScaling::LaunchConfiguration`).
 
+The `resourceIds` dimension matches only diagnostics attributed to a resource; `logicalIds` additionally matches
+diagnostics on parameters, outputs, mappings, conditions, and template rules (for resource diagnostics the two carry
+the same value). An optional `entityType` scopes a `LogicalIdFilter` to entities of one type, so `MyThing` as a
+`"Parameter"` is matched without touching a same-named entity of another type.
+
 ### PseudoParameterOverrides
 
 Override CloudFormation pseudo-parameters used during intrinsic function resolution. All fields optional — when
@@ -200,7 +207,7 @@ interface StandardReport {
 ```
 
 `DetailedReport` has the same structure but its diagnostics include additional fields: `documentationUrl`,
-`ruleDescription`, `phase` (`PARSE` | `SCHEMA` | `LINT`), `section`, and `context` (`ViolationContext` with
+`ruleDescription`, `phase` (`PARSE` | `SCHEMA` | `LINT`), and `context` (`ViolationContext` with
 `actualValue`, `expectedConstraint`, `resolutionSource`, etc.).
 
 ### StandardDiagnostic
@@ -211,9 +218,8 @@ interface StandardDiagnostic {
     severity: Severity;                // "FATAL" | "ERROR" | "WARN" | "INFO" | "DEBUG"
     message: string;
     source: RuleOrigin;                // "SCHEMA" | "CFN_LINT" | "ENGINE" | "CUSTOM" | "GUARD"
-    resourceId?: string;               // logical resource ID
-    resourceType?: string;             // e.g. "AWS::S3::Bucket"
-    propertyPath?: string;             // e.g. "Properties/BucketName"
+    entity?: Entity;                   // the named template entity the finding targets, if any
+    propertyPath?: string;             // e.g. "Properties.BucketName", or section-absolute like "Parameters/MyParam/Type"
     suggestedFix?: string;
     category?: string;
     startLine?: number;
@@ -223,4 +229,15 @@ interface StandardDiagnostic {
     relatedResources?: RelatedResource[];
     conditionScenario?: Record<string, boolean>;  // condition truth assignment that triggers this diagnostic
 }
+
+// The named template entity a diagnostic is attributed to. The entity type is the
+// singular form of the top-level template section the entity is declared in.
+interface Entity {
+    logicalId: string;                 // logical ID as declared in the template
+    entityType: EntityType;
+    resourceType?: string;             // CloudFormation type, when the entity is a resource whose type is known
+}
+
+type EntityType = "Resource" | "Parameter" | "Output" | "Mapping" | "Metadata"
+                | "Rule" | "Condition" | "Transform" | "FormatVersion" | "Description";
 ```

src/bindings-jvm/README.md

@@ -19,11 +19,14 @@ export type {
     RuleOrigin,
     IdRange,
     ResourceIdFilter,
+    LogicalIdFilter,
     ResourceTypeFilter,
     ServiceFilter,
     RuleFilterConfig,
     RuleInfo,
     SourceSpan,
+    Entity,
+    EntityType,
     ResourceRef,
     RelatedResource,
     ViolationContext,

src/bindings-wasm/README.md

@@ -2,7 +2,7 @@ use log::info;
 use std::collections::HashMap;
 use std::sync::Arc;
 
-use diagnostics::{Diagnostic, PhaseMetric, ResourceRef, UNKNOWN_SPAN, phase_metric};
+use diagnostics::{Diagnostic, Entity, PhaseMetric, UNKNOWN_SPAN, phase_metric};
 use guard_translator::{ensure_translatable, pack_name_from_path, parse_guard};
 use rules::{RuleInfo, RuleMetadataEntry, RuleOrigin, Severity, build_rule_metadata_map, is_valid_custom_rule_id};
 use template_model::SemanticModel;
@@ -251,14 +251,7 @@ fn emit_custom_diagnostic(
         rule_id: rule.rule_id.clone(),
         severity: rule.severity,
         message: msg.to_string(),
-        resource: if rid.is_empty() {
-            None
-        } else {
-            Some(ResourceRef {
-                id: Some(rid.to_string()),
-                resource_type: model.resources.get(rid).map(|r| r.resource_type.clone()),
-            })
-        },
+        entity: Entity::resource(rid, model.resources.get(rid).map(|r| r.resource_type.clone())),
         property_path: rule.prop_path.clone(),
         suggested_fix: rule.suggested_fix.clone(),
         documentation_url: None,
@@ -268,7 +261,6 @@ fn emit_custom_diagnostic(
         condition_scenario: None,
         rule_description: None,
         phase: None,
-        section: None,
         context: None,
         source: rule.source,
     });

src/bindings-wasm/ts/index.ts

@@ -360,7 +360,7 @@ fn eval_best_practices(ctx: &EvalContext) -> Vec<Diagnostic> {
                         &format!("Parameter {} used as {}, therefore NoEcho should be True", target, prop),
                         m,
                         "",
-                        &format!("Parameters.{}", target),
+                        &format!("Parameters/{}", target),
                         None,
                     ));
                 }

src/cel-engine/src/engine.rs

@@ -131,16 +131,17 @@ fn eval_unreachable_if_branches(ctx: &EvalContext) -> Vec<Diagnostic> {
     }
 
     // Also check Output values for unreachable Fn::If branches. An output is not
-    // a resource, so anchor the diagnostic at the full "Outputs/<name>/Value"
-    // path (matching how output locations are addressed elsewhere) rather than a
-    // bare "Value" under the output's logical name.
+    // a resource, so leave the resource slot empty and anchor the diagnostic at
+    // the full "Outputs/<name>/Value" path (matching how output locations are
+    // addressed elsewhere) rather than a bare "Value" under the output's logical
+    // name.
     for (name, output) in &m.outputs {
         let base_assumptions: Vec<(String, bool)> = match &output.condition {
             Some(cond) => vec![(cond.clone(), true)],
             None => vec![],
         };
         let path_prefix = format!("Outputs/{}/Value", name);
-        find_unreachable_branches(&mut out, m, name, &output.value, &path_prefix, &base_assumptions);
+        find_unreachable_branches(&mut out, m, "", &output.value, &path_prefix, &base_assumptions);
     }
     out
 }