Merge pull request #4084 from icsharpcode/fix/field-keyword-gating

Fix the C# 14 field keyword being printed next to a surviving backing field

ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -264,6 +264,8 @@
     <None Include="TestCases\Ugly\AggressiveScalarReplacementOfAggregates.Expected.cs" />
     <Compile Remove="TestCases\Ugly\NoArrayInitializers.Expected.cs" />
     <None Include="TestCases\Ugly\NoArrayInitializers.Expected.cs" />
+    <Compile Remove="TestCases\Ugly\NoAutomaticProperties.Expected.cs" />
+    <None Include="TestCases\Ugly\NoAutomaticProperties.Expected.cs" />
     <Compile Remove="TestCases\Ugly\NoDecimalConstants.Expected.cs" />
     <None Include="TestCases\Ugly\NoDecimalConstants.Expected.cs" />
     <Compile Remove="TestCases\Ugly\NoExtensionMethods.Expected.cs" />
@@ -272,6 +274,8 @@
     <None Include="TestCases\Ugly\NoFieldKeyword.Expected.cs" />
     <Compile Remove="TestCases\Ugly\NoForEachStatement.Expected.cs" />
     <None Include="TestCases\Ugly\NoForEachStatement.Expected.cs" />
+    <Compile Remove="TestCases\Ugly\NoGetterOnlyAutomaticProperties.Expected.cs" />
+    <None Include="TestCases\Ugly\NoGetterOnlyAutomaticProperties.Expected.cs" />
     <Compile Remove="TestCases\Ugly\NoInitAccessors.Expected.cs" />
     <None Include="TestCases\Ugly\NoInitAccessors.Expected.cs" />
     <Compile Remove="TestCases\Ugly\NoLocalFunctions.Expected.cs" />

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);
+		}
+	}
+}

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);
+			}
+		}
+	}
+}

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;
+	}
+}

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;
+		}
+	}
+}

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)
 		{

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

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;
 		}
 
 		/// <summary>
@@ -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<MemberResolveResult>();
+			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<MemberResolveResult>();
-				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<MemberResolveResult>();
-					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<MemberResolveResult>();
+			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
 		/// </summary>
 		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);
 		}