Modify S1104: Migrate To LayC - avoid public field (#3238)

rules/S1104/csharp/rule.adoc

@@ -1,47 +1,75 @@
 == Why is this an issue?
 
-Public fields in public classes do not respect the encapsulation principle and has three main disadvantages:
+include::../description.adoc[]
 
-* Additional behavior such as validation cannot be added.
-* The internal representation is exposed, and cannot be changed afterwards.
-* Member values are subject to change from anywhere in the code and may not meet the programmer's assumptions.
+Note that due to optimizations on simple properties, public fields provide only very little performance gain.
+
+include::../impacts.adoc[]
 
-By using private fields and public properties (set and get), unauthorized modifications are prevented. Properties also benefit from additional protection (security) features such as Link Demands.
+=== Exceptions
 
+Fields marked as `readonly` or `const` are ignored by this rule.
 
-Note that due to optimizations on simple properties, public fields provide only very little performance gain.
+Fields inside classes or structs annotated with the `StructLayoutAttribute` are ignored by this rule.
+
+== How to fix it
+
+Depending on your needs:
+
+* Use auto-implemented properties: +
+For common cases, where no validation is required, auto-implemented properties are a good alternative to fields: these allows fine grained access control and offers the flexibility to add validation or change internal storage afterwards. 
+__Note:__ as a bonus it is now possible to monitor value changes using breakpoints.
 
-=== Noncompliant code example
+* Encapsulate the fields in your class. To do so:
 
-[source,csharp]
+ . Make the field private.
+ . Use public properties (set and get) to access and modify the field.
+
+* Mark field as `readonly` or `const`.
+
+=== Code examples
+
+==== Noncompliant code example
+
+[source,csharp,diff-id=1,diff-type=noncompliant]
 ----
 public class Foo
 {
-    public int instanceData = 32; // Noncompliant
+    public int InstanceData = 32; // Noncompliant
+    public int AnotherInstanceData = 32; // Noncompliant
+    
 }
 ----
 
-=== Compliant solution
+==== Compliant solution
 
-[source,csharp]
+[source,csharp,diff-id=1,diff-type=compliant]
 ----
 public class Foo
 {
-    private int instanceData = 32;
+    // using auto-implemented properties
+    public int InstanceData { get; set; } = 32;
 
-    public int InstanceData
+    // using field encapsulation
+    private int _anotherInstanceData = 32;
+
+    public int AnotherInstanceData
     {
-        get { return instanceData; }
-	set { instanceData = value ; }
+        get { return _anotherInstanceData; }
+        set
+        {
+            // perform validation
+            _anotherInstanceData = value;
+        }
     }
+
 }
 ----
 
-=== Exceptions
+=== Pitfalls
 
-Fields marked as ``++readonly++`` or ``++const++`` are ignored by this rule.
+Please be aware that changing a field by a property in a software that uses serialization could lead to binary incompatibility.
 
-Fields inside classes or structs annotated with the ``++StructLayoutAttribute++`` are ignored by this rule.
 
 include::../see.adoc[]
 

rules/S1104/description.adoc

@@ -1,8 +1,8 @@
-Public class variable fields do not respect the encapsulation principle and has three main disadvantages:
+Public fields in public classes do not respect the encapsulation principle and have three main disadvantages:
 
 
 * Additional behavior such as validation cannot be added.
 * The internal representation is exposed, and cannot be changed afterwards.
 * Member values are subject to change from anywhere in the code and may not meet the programmer's assumptions.
 
-By using private attributes and accessor methods (set and get), unauthorized modifications are prevented.
+To prevent unauthorized modifications, private attributes and accessor methods (set and get) should be used.

rules/S1104/impacts.adoc

@@ -0,0 +1,7 @@
+=== What is the potential impact?
+
+Public fields can be modified by any part of the code and this can lead to unexpected changes and hard-to-trace bugs.
+
+Public fields don't hide the implementation details. As a consequence, it is no longer possible to change how the data is stored internally without impacting the client code of the class.
+
+The code is harder to maintain.

rules/S1104/java/rule.adoc

@@ -2,13 +2,65 @@
 
 include::../description.adoc[]
 
-include::../noncompliant.adoc[]
+include::../impacts.adoc[]
 
-include::../compliant.adoc[]
 
 === Exceptions
 
-Because they are not modifiable, this rule ignores ``++public final++`` fields. Also, annotated fields, whatever the annotation(s) will be ignored, as annotations are often used by injection frameworks, which in exchange require having public fields.
+This rule ignores `public final` fields because they are not modifiable. Also, annotated fields, whatever the annotation(s) will be ignored, as annotations are often used by injection frameworks, which in exchange require having public fields.
+
+== How to fix it
+
+Depending on your need there are multiple options:
+
+* Encapsulate the field
+    . Make the field private.
+    . Define methods to get and set the value of the field. +
+    These methods are commonly known as getter and setter methods and are prefixed by `get` and `set` followed by the name of the field. __Note:__ as a bonus it is now possible to monitor value changes using breakpoints.
+* Mark the field as `public final` if it is not supposed to change.
+
+=== Code examples
+
+==== Noncompliant code example
+
+[source,java,diff-id=1,diff-type=noncompliant]
+----
+public class MyClass {
+
+  public static final int SOME_CONSTANT = 0;     // Compliant - constants are not checked
+
+  public String firstName;                       // Noncompliant
+
+}
+----
+
+==== Compliant solution
+
+[source,java,diff-id=1,diff-type=compliant]
+----
+public class MyClass {
+
+  public static final int SOME_CONSTANT = 0;     // Compliant - constants are not checked
+
+  private String firstName;
+
+  public String getFirstName() {
+    return firstName;
+  }
+
+  public void setFirstName(String firstName) {
+    this.firstName = firstName;
+  }
+
+}
+----
+
+=== How does this work?
+
+By having a setter and a getter the code can control how the field is accessed and modified. 
+For example, adding validation in the setter method will ensure that only valid values are set.
+
+The access modifiers on the setter can also be changed to `private` or `protected` to restrain which code can modify the value.
 
 include::../see.adoc[]