diff --git a/ICSharpCode.Decompiler.Generators/AnalyzerReleases.Unshipped.md b/ICSharpCode.Decompiler.Generators/AnalyzerReleases.Unshipped.md new file mode 100644 index 000000000..a0353a503 --- /dev/null +++ b/ICSharpCode.Decompiler.Generators/AnalyzerReleases.Unshipped.md @@ -0,0 +1,8 @@ +; Unshipped analyzer release +; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +DSTG001 | DecompilerSyntaxTreeGenerator | Error | Slot kind must map to a single child type diff --git a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs index 150214591..00d214ea7 100644 --- a/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs +++ b/ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs @@ -30,6 +30,20 @@ namespace ICSharpCode.Decompiler.Generators; [Generator] internal class DecompilerSyntaxTreeGenerator : IIncrementalGenerator { + // Enforces the slot-model invariant: a slot kind names one child position, so it must map to a + // single declared child type across all nodes that use it. A kind used with two different types + // would have to widen its typed Slots constant to AstNode, which makes the typed child accessors + // (GetChildren in particular) unable to recover the real element type and throw at runtime. A + // position that is genuinely either an expression or a statement (a lambda body) is still one + // declared type -- AstNode -- and is fine; the error fires only on coincidental name reuse. + static readonly DiagnosticDescriptor MultipleChildTypesForKind = new( + id: "DSTG001", + title: "Slot kind used with multiple child types", + messageFormat: "Slot kind '{0}' is declared with multiple child types ({1}); each [Slot] kind must map to a single child type. Give the differing position its own [Slot] name.", + category: "DecompilerSyntaxTreeGenerator", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true); + record AstNodeAdditions(string NodeName, bool NeedsVisitor, bool IsAbstract, bool BaseHasDefaultConstructor, bool NeedsPatternPlaceholder, string VisitMethodName, string VisitMethodParamType, EquatableArray? MembersToMatch, EquatableArray? Slots, EquatableArray? NameAccessors, EquatableArray? CtorParams); // One DoMatch comparison: Member is the property (or the "MatchAttributesAndModifiers" sentinel); @@ -801,20 +815,25 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax } } + // A slot kind names one child position, so it maps to a single child type (DSTG001 enforces this); + // the typed Slots constant therefore always carries that precise type. A kind that is a collection + // on one node and a single (or optional) child on another keeps a single type but no single arity, + // so its hard-coded IsCollection/isOptional flags are not authoritative -- the precise per-position + // flags live on the per-node slots, which is where consumers read them; the shared constant carries + // identity (and the now-precise child type), not those flags. + foreach (var kv in kindTypes) + { + if (kv.Value.Count > 1) + context.ReportDiagnostic(Diagnostic.Create(MultipleChildTypesForKind, Location.None, kv.Key, string.Join(", ", kv.Value))); + } + var builder = new StringBuilder(); builder.AppendLine("// "); builder.AppendLine(); builder.AppendLine("namespace ICSharpCode.Decompiler.CSharp.Syntax;"); builder.AppendLine(); // Each constant is its own canonical kind (null Kind): node.GetChild(Slots.X) infers the child - // type, and node.Slot.Kind == Slots.X identifies a position polymorphically. A kind reused with - // several child types across node types widens to AstNode (only its concrete per-node slots carry - // the precise type); such kinds are only reached through those per-node slots. For the same reason - // the shared kind's IsCollection flag and its hard-coded isOptional: false are not authoritative - // when a kind is a collection on one node and a single/optional child on another: such dual-use - // kinds carry IsCollection false (no arity claim), and the precise per-position flags live on the - // per-node slots, which is where consumers read them; the shared constant carries identity, not - // those flags. + // type, and node.Slot.Kind == Slots.X identifies a position polymorphically. builder.AppendLine("/// The shared slot kinds, one per distinct child position across the AST node types."); builder.AppendLine("public static class Slots"); builder.AppendLine("{"); diff --git a/ICSharpCode.Decompiler.Generators/ICSharpCode.Decompiler.Generators.csproj b/ICSharpCode.Decompiler.Generators/ICSharpCode.Decompiler.Generators.csproj index 0449c2168..2f0a81ddb 100644 --- a/ICSharpCode.Decompiler.Generators/ICSharpCode.Decompiler.Generators.csproj +++ b/ICSharpCode.Decompiler.Generators/ICSharpCode.Decompiler.Generators.csproj @@ -18,4 +18,9 @@ + + + + + diff --git a/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs b/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs index 4970a2f57..1b88beabc 100644 --- a/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs +++ b/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs @@ -225,7 +225,7 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor bool SkipToken() { - return kind == Slots.Initializer + return kind == Slots.ForInitializer || kind == Slots.Iterator || kind == Slots.ResourceAcquisition; } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/LambdaExpression.cs b/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/LambdaExpression.cs index e4dea191a..feccf3443 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/LambdaExpression.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/LambdaExpression.cs @@ -44,7 +44,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("Parameter")] public partial AstNodeCollection Parameters { get; } - [Slot("Body")] + [Slot("LambdaBody")] public partial AstNode Body { get; set; } } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/SwitchExpression.cs b/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/SwitchExpression.cs index 4ecece918..e67b5ae32 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/SwitchExpression.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/Expressions/SwitchExpression.cs @@ -32,7 +32,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("Expression")] public partial Expression Expression { get; set; } - [Slot("SwitchSection")] + [Slot("SwitchExpressionSection")] public partial AstNodeCollection SwitchSections { get; } } @@ -45,7 +45,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("Pattern")] public partial Expression Pattern { get; set; } - [Slot("Body")] + [Slot("SwitchExpressionBody")] public partial Expression Body { get; set; } } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/README.md b/ICSharpCode.Decompiler/CSharp/Syntax/README.md index 2e74694cb..2cc8256c9 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/README.md +++ b/ICSharpCode.Decompiler/CSharp/Syntax/README.md @@ -60,10 +60,13 @@ and the **string** is the slot *kind* (see [Slots and kinds](#slots-and-kinds)): The kind string is shared across node types: every slot tagged `[Slot("Expression")]` has the same kind, so `node.Slot.Kind == Slots.Expression` works polymorphically. Pick an existing kind name when -the position is the same logical role as on other nodes; pick a new one when it differs. Keep a kind -mapping to a single child type -where you can -- if one kind is used with several child types its typed `Slots` constant widens to -`AstNode` (still usable, but you lose inference for that kind; see the de-aliasing note below). +the position is the same logical role *and* child type as on other nodes; pick a new one when either +differs. A kind must map to a **single child type** across every node that uses it: the generator +emits one typed `Slots.X` constant per kind, and a kind spanning two child types would have to widen +that constant to `AstNode`, leaving the typed accessors (`GetChildren` especially) unable to recover +the real element type. The generator enforces this at build time (`DSTG001`). A position that is +genuinely an expression *or* a statement -- a lambda body -- is still a single declared type, +`AstNode`, and is fine. ### `[ExcludeFromMatch]` @@ -140,9 +143,12 @@ So a transform that corrupts the tree fails at the exact transform, not as a dow ## Conventions and gotchas - The `[Slot]` string is the **bare kind name** ("Body", "Expression"), not a dotted role expression. -- A kind reused with two different child types widens its `Slots` constant to `AstNode`. If you need a - precise type, give the position its own kind name -- as the declaration-level attribute slot does - (`[Slot("AttributeSection")]`) versus an attribute section's own `[Slot("Attribute")]`. +- A kind maps to exactly one child type; reusing one name for two different child types is a build + error (`DSTG001`). When two positions would share a name but differ in child type, give each its own + kind name -- as the declaration-level attribute slot (`[Slot("AttributeSection")]`) does versus an + attribute section's own `[Slot("Attribute")]`, or the for-statement initializer + (`[Slot("ForInitializer")]`, a `Statement` collection) versus an object/array initializer + (`[Slot("Initializer")]`, an `ArrayInitializerExpression`). - Optional **names** read back as `null` (not an empty string) and a `null`/empty assignment clears the backing token. - Don't hand-write what the generator emits (visitors, `DoMatch`, child dispatch, constructors, slot diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/Statements/ForStatement.cs b/ICSharpCode.Decompiler/CSharp/Syntax/Statements/ForStatement.cs index fa54913fc..ac1d25b56 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/Statements/ForStatement.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/Statements/ForStatement.cs @@ -41,7 +41,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax /// Note: this contains multiple statements for "for (a = 2, b = 1; a > b; a--)", but contains /// only a single statement for "for (int a = 2, b = 1; a > b; a--)" (a single VariableDeclarationStatement with two variables) /// - [Slot("Initializer")] + [Slot("ForInitializer")] public partial AstNodeCollection Initializers { get; } [Slot("Condition")] diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/Statements/IfElseStatement.cs b/ICSharpCode.Decompiler/CSharp/Syntax/Statements/IfElseStatement.cs index 11d7f7784..08cfa252c 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/Statements/IfElseStatement.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/Statements/IfElseStatement.cs @@ -40,10 +40,10 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("Condition")] public partial Expression Condition { get; set; } - [Slot("True")] + [Slot("TrueStatement")] public partial Statement TrueStatement { get; set; } - [Slot("False")] + [Slot("FalseStatement")] public partial Statement? FalseStatement { get; set; } } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/ConstructorDeclaration.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/ConstructorDeclaration.cs index f79bc079f..5af8cc147 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/ConstructorDeclaration.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/ConstructorDeclaration.cs @@ -51,7 +51,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("Parameter")] public partial AstNodeCollection Parameters { get; } - [Slot("Initializer")] + [Slot("ConstructorInitializer")] public partial ConstructorInitializer? Initializer { get; set; } [Slot("Body")] diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EnumMemberDeclaration.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EnumMemberDeclaration.cs index fa9324f1e..5d5945817 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EnumMemberDeclaration.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/EnumMemberDeclaration.cs @@ -46,7 +46,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("Identifier")] public override partial Identifier NameToken { get; set; } - [Slot("Initializer")] + [Slot("EnumMemberInitializer")] public partial Expression? Initializer { get; set; } } } diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/FixedFieldDeclaration.cs b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/FixedFieldDeclaration.cs index e5e02396b..7ea63ef46 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/FixedFieldDeclaration.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/TypeMembers/FixedFieldDeclaration.cs @@ -47,7 +47,7 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax [Slot("Type")] public override partial AstType ReturnType { get; set; } - [Slot("Variable")] + [Slot("FixedVariable")] public partial AstNodeCollection Variables { get; } } } diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs index 5f231aba2..d9ae39453 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs @@ -587,7 +587,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (v.Type.IsByRefLike) return true; // by-ref-like variables always must be initialized at their declaration. - if (v.InsertionPoint.nextNode.Slot?.Kind == Slots.Initializer) + if (v.InsertionPoint.nextNode.Slot?.Kind == Slots.ForInitializer) return true; // for-statement initializers always should combine declaration and initialization. return !context.Settings.SeparateLocalVariableDeclarations; diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs b/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs index 394c4fde1..b0774e941 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/NormalizeBlockStatements.cs @@ -117,7 +117,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms bool IsElseIf(Statement statement, Statement parent) { - return parent is IfElseStatement && statement.Slot?.Kind == Slots.False; + return parent is IfElseStatement && statement.Slot?.Kind == Slots.FalseStatement; } static void InsertBlock(Statement statement) @@ -142,7 +142,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms switch (statement) { case IfElseStatement ies: - return parent is IfElseStatement && ies.Slot?.Kind == Slots.False; + return parent is IfElseStatement && ies.Slot?.Kind == Slots.FalseStatement; case VariableDeclarationStatement vds: case WhileStatement ws: case DoWhileStatement dws: diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs index 2e1e07db8..2aa550794 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs @@ -192,7 +192,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (next is ForStatement forStatement && ForStatementUsesVariable(forStatement, variable)) { node.Remove(); - next.InsertChildAfter(null, node, Slots.Initializer); + next.InsertChildAfter(null, node, Slots.ForInitializer); return (ForStatement)next; } Match m3 = forPattern.Match(next);