diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index 714454f75..c385749ce 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -264,6 +264,8 @@ + + @@ -272,6 +274,8 @@ + + diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoAutomaticProperties.Expected.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoAutomaticProperties.Expected.cs new file mode 100644 index 000000000..85f7feaab --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoAutomaticProperties.Expected.cs @@ -0,0 +1,38 @@ +using System; +using System.Runtime.CompilerServices; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly; + +internal class NoAutomaticProperties +{ + public int Plain { + [CompilerGenerated] + get { + return field; + } + [CompilerGenerated] + set { + field = value; + } + } + + public int WithInitializer { + [CompilerGenerated] + get { + return field; + } + [CompilerGenerated] + set { + field = value; + } + } = 5; + + public int SemiAuto { + get { + return field; + } + set { + field = Math.Max(0, value); + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoAutomaticProperties.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoAutomaticProperties.cs new file mode 100644 index 000000000..b1ca4aaee --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoAutomaticProperties.cs @@ -0,0 +1,38 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly +{ + internal class NoAutomaticProperties + { + public int Plain { get; set; } + + public int WithInitializer { get; set; } = 5; + + public int SemiAuto { + get { + return field; + } + set { + field = Math.Max(0, value); + } + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoGetterOnlyAutomaticProperties.Expected.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoGetterOnlyAutomaticProperties.Expected.cs new file mode 100644 index 000000000..6767427b2 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoGetterOnlyAutomaticProperties.Expected.cs @@ -0,0 +1,29 @@ +#if !OPT +using System.Diagnostics; +#endif +using System.Runtime.CompilerServices; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly; + +internal class NoGetterOnlyAutomaticProperties +{ + [CompilerGenerated] +#if !OPT + [DebuggerBrowsable(DebuggerBrowsableState.Never)] +#endif + private readonly int GetOnly__BackingField; + + public int GetOnly { + [CompilerGenerated] + get { + return GetOnly__BackingField; + } + } + + public int WithSetter { get; set; } + + public NoGetterOnlyAutomaticProperties() + { + GetOnly__BackingField = 5; + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoGetterOnlyAutomaticProperties.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoGetterOnlyAutomaticProperties.cs new file mode 100644 index 000000000..c2d46690f --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoGetterOnlyAutomaticProperties.cs @@ -0,0 +1,32 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly +{ + internal class NoGetterOnlyAutomaticProperties + { + public int GetOnly { get; } + + public int WithSetter { get; set; } + + public NoGetterOnlyAutomaticProperties() + { + GetOnly = 5; + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs b/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs index c89807843..15127eb5c 100644 --- a/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs @@ -128,6 +128,22 @@ namespace ICSharpCode.Decompiler.Tests }); } + [Test] + public async Task NoAutomaticProperties([ValueSource(nameof(roslynLatestOnlyOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings { + AutomaticProperties = false + }); + } + + [Test] + public async Task NoGetterOnlyAutomaticProperties([ValueSource(nameof(roslynLatestOnlyOptions))] CompilerOptions cscOptions) + { + await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings { + GetterOnlyAutomaticProperties = false + }); + } + [Test] public async Task NoFieldKeyword([ValueSource(nameof(roslynLatestOnlyOptions))] CompilerOptions cscOptions) { diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index ec1be0aa4..953e46c56 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -331,25 +331,10 @@ namespace ICSharpCode.Decompiler.CSharp return eventReference.WithRR(eventResolveResult); } - if (settings.FieldKeyword - && decompilationContext.CurrentMember is IProperty accessedProperty - && accessedProperty.Parameters.Count == 0 - // Ask exactly the question PatternStatementTransform asks when it decides whether - // the field declaration can go away. A looser test here prints `field` inside a - // property whose declaration then keeps explicit accessors and its field: on - // recompile the keyword binds to a freshly synthesized backing field while the - // original one stays declared and unwritten - silently different storage. - && PatternStatementTransform.TryGetBackingField(accessedProperty, out var backingField) - && field.MemberDefinition.Equals(backingField.MemberDefinition) - // Only THIS instance's field is the `field` keyword. IL can load another - // instance's backing field inside an accessor (weavers, obfuscators, hand-written - // IL); rendering that as `field` would redirect the access, and drop whatever - // side effect producing the target had. - && (field.IsStatic || TargetIsThis(targetInstruction))) - { - // Inside its own property's get/set/init accessor (including nested lambdas and - // local functions), the backing field is the C# 14 "field" keyword. It must stay - // unqualified: "this.field" would refer to a real member named "field". + if (CanUseFieldKeyword()) + { + // The keyword must stay unqualified: "this.field" would refer to a real member + // named "field". return new IdentifierExpression("field") .WithRR(new MemberResolveResult(null, field)); } @@ -441,6 +426,38 @@ namespace ICSharpCode.Decompiler.CSharp } return expr; + + // Whether this access may be rendered as the C# 14 "field" keyword: it has to be the + // backing field of the property whose accessor is being decompiled, read off this + // instance, in a property the declaration can actually disappear from. Nested lambdas + // and local functions inside the accessor count as being inside it. + bool CanUseFieldKeyword() + { + if (!settings.FieldKeyword) + return false; + if (decompilationContext.CurrentMember is not IProperty property || property.Parameters.Count != 0) + return false; + // With GetterOnlyAutomaticProperties off, a setter-less property keeps its backing + // field declared (CSharpDecompiler.MemberIsHidden) and PatternStatementTransform + // leaves the property alone, so the keyword would land next to the declaration it + // is supposed to replace. + if (!property.CanSet && !settings.GetterOnlyAutomaticProperties) + return false; + // Exactly the question PatternStatementTransform asks before removing the + // declaration. A looser test prints "field" in a property that then keeps its + // field: on recompile the keyword binds to a freshly synthesized backing field + // while the original stays declared and unwritten - silently different storage. + if (!PatternStatementTransform.TryGetBackingField(property, out var backingField) + || !field.MemberDefinition.Equals(backingField.MemberDefinition)) + { + return false; + } + // Only THIS instance's field is the keyword. IL can load another instance's backing + // field inside an accessor (weavers, obfuscators, hand-written IL); rendering that + // as "field" would redirect the access and drop whatever side effect produced the + // target. + return field.IsStatic || TargetIsThis(targetInstruction); + } } // References to an automatic event's backing field are printed as the event. Gated on diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs index 2f4adf6e4..d84150701 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs @@ -111,7 +111,13 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms public override AstNode VisitPropertyDeclaration(PropertyDeclaration propertyDeclaration) { - if (context.Settings.AutomaticProperties + // Same rule as CSharpDecompiler.MemberIsHidden applies to the backing field: either + // setting on its own allows the field declaration to disappear, and + // GetterOnlyAutomaticProperties vetoes the getter-only case for both. Asking only + // about AutomaticProperties would skip the transform for a field-backed property + // while ExpressionBuilder.ConvertField has already printed "field" in its accessors, + // leaving the declaration and the keyword in the same output. + if ((context.Settings.AutomaticProperties || context.Settings.FieldKeyword) && (propertyDeclaration.Setter is not null || context.Settings.GetterOnlyAutomaticProperties)) { AstNode? result = TransformAutomaticProperty(propertyDeclaration); @@ -1083,12 +1089,20 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms property = null; if (!NameCouldBeBackingFieldOfAutomaticProperty(field.Name, out var propertyName)) return false; - if (!field.IsCompilerGenerated()) - return false; - property = field.DeclaringTypeDefinition? + var candidate = field.DeclaringTypeDefinition? .GetProperties(p => p.Name == propertyName, GetMemberOptions.IgnoreInheritedMembers) .FirstOrDefault(); - return property != null; + // Answering through TryGetBackingField keeps the two directions of the same question + // from disagreeing: it is the predicate every transform consults before removing a + // declaration, and it checks more than the name (compiler-generated, staticness and + // field type all have to match the property). + if (candidate == null || !TryGetBackingField(candidate, out var backingField) + || !field.MemberDefinition.Equals(backingField.MemberDefinition)) + { + return false; + } + property = candidate; + return true; } /// @@ -1113,45 +1127,47 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms Identifier? ReplaceBackingFieldUsage(Identifier identifier) { - if (NameCouldBeBackingFieldOfAutomaticProperty(identifier.Name, out _)) + // The resolve result, not the spelling, identifies the member: after + // ExpressionBuilder.ConvertField the same backing field appears both as the C# 14 "field" + // keyword and under its metadata name, carrying the same annotation either way. + var parent = identifier.Parent; + if (parent == null) + return null; + var mrr = parent.Annotation(); + if (mrr?.Member is not IField field || !IsBackingFieldOfAutomaticProperty(field, out var property) + || currentMethod?.AccessorOwner == property) { - var parent = identifier.Parent; - if (parent == null) + return null; + } + // With the keyword available TransformAutomaticProperty routes every property through + // TransformFieldBackedProperty, so its verdict on the declaration is the only one that + // counts: while the field stays declared, a field reference remains the correct - and + // only compilable - rendering. + if (context.Settings.FieldKeyword && !BackingFieldWillBeRemoved(property, field, parent)) + return null; + if (context.Settings.AutomaticProperties + && CanTransformToAutomaticProperty(property, !(field.IsCompilerGenerated() && field.Name == "_" + property.Name))) + { + if (!property.CanSet && !context.Settings.GetterOnlyAutomaticProperties) return null; - var mrr = parent.Annotation(); - if (mrr?.Member is IField field && IsBackingFieldOfAutomaticProperty(field, out var property) - && currentMethod?.AccessorOwner != property) - { - if (CanTransformToAutomaticProperty(property, !(field.IsCompilerGenerated() && field.Name == "_" + property.Name))) - { - if (!property.CanSet && !context.Settings.GetterOnlyAutomaticProperties && !context.Settings.FieldKeyword) - return null; - } - else if (context.Settings.FieldKeyword && !property.CanSet && IsConstructorStoreTarget(parent, field) - && BackingFieldWillBeRemoved(property, field, parent)) - { - // A direct store to the backing field of a setter-less field-backed - // property is expressible as a property assignment in a constructor - - // but only where the property declaration actually becomes field-backed. - // If TransformFieldBackedProperty bails, the property keeps explicit - // accessors and no setter, so assigning it would not compile (CS0200). - } - else - { - // Stores that initialize a field-backed property with a setter are left - // as field references and lifted into the property initializer by - // TransformFieldAndConstructorInitializers (a property assignment would - // invoke the setter); everything else is inexpressible with the "field" - // keyword and keeps the field declared. - return null; - } - context.Step("Replace backing field use with property", identifier); - parent.RemoveAnnotations(); - parent.AddAnnotation(new MemberResolveResult(mrr.TargetResult, property)); - return Identifier.Create(property.Name); - } } - return null; + else if (context.Settings.FieldKeyword && !property.CanSet && IsConstructorStoreTarget(parent, field)) + { + // A setter-less field-backed property keeping explicit accessors is still assignable + // by name inside a constructor of its declaring type, and that is the only form the + // store has left once the declaration is gone. + } + else + { + // The property keeps explicit accessors and a setter, so its name would invoke that + // setter. The store stays a field reference and TransformFieldAndConstructorInitializers + // lifts it into the property initializer, which is what field-backed storage means. + return null; + } + context.Step("Replace backing field use with property", identifier); + parent.RemoveAnnotations(); + parent.AddAnnotation(new MemberResolveResult(mrr.TargetResult, property)); + return Identifier.Create(property.Name); } bool IsConstructorStoreTarget(AstNode node, IField field) @@ -1164,7 +1180,11 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms /// bool BackingFieldWillBeRemoved(IProperty property, IField field, AstNode nodeInTree) { - return TryGetBackingField(property, out var backingField) + return context.Settings.FieldKeyword + // The same gate VisitPropertyDeclaration applies: a setter-less property is only + // transformed where getter-only auto-properties are allowed. + && (property.CanSet || context.Settings.GetterOnlyAutomaticProperties) + && TryGetBackingField(property, out var backingField) && field.MemberDefinition.Equals(backingField.MemberDefinition) && OutsideReferencesAreExpressible(nodeInTree, backingField); }