Property Getters should be pure

Rule ND1904 Immutability

A property getter with side-effects is unexpected and counterintuitive for callers: getters should be pure, assigning no field.

To avoid matching lazy initialization at first access (an invisible implementation detail for clients), getters assigning a field only assigned by themselves and the corresponding setter are not matched, nor are fields with a name similar to the property name.

How to fix

Make matched property getters assign no field.

Estimated Debt: 2 minutes plus 5 minutes per field assigned and 5 minutes per other method assigning such field.

CQLinq source

This rule is written in CQLinq, C# LINQ over the NDepend code model. It ships with every new NDepend project: you can read it, adapt it, or use it as a starting point for your own custom rules.

// <Name>Property Getters should be pure</Name>
// <Id>ND1904:PropertyGettersShouldBeImmutable</Id>

warnif count > 0 from m in Application.Methods where
 !m.IsGeneratedByCompiler &&
  m.IsPropertyGetter &&
  m.ParentProperty != null &&
  ( ( !m.IsStatic && m.ChangesObjectState) ||
    (  m.IsStatic && m.ChangesTypeState) )

let propertyName = m.ParentProperty.SimpleName

let fieldsAssigned = m.FieldsAssigned.Where(f => 

          // Don't count field that have a name similar to the property name
          // to avoid matching lazy initialization situations.
         !(propertyName.Length >= 4 &&
           f.SimpleName.Length >= 4 &&
           // Don't count the first 3 characters of the field name, 
           // to avoid special field name formatting like 'm_X' or '_x'
           propertyName.EndsWith(f.SimpleName.Substring(3, f.SimpleName.Length -3))) 
          &&

          // Don't count field that are assigned only by the property getter and the related property setter.          
          f.MethodsAssigningMe.Any(m1 => m1 != m && 
                                       !(m1.IsPropertySetter && m1.ParentProperty == m1.ParentProperty)))
where fieldsAssigned.Any()
let otherMethodsAssigningSameFields = fieldsAssigned.SelectMany(f => f.MethodsAssigningMe.Where(m1 => m1 != m))

select new {
   m, 
   m.NbLinesOfCode, 
   fieldsAssigned,
   otherMethodsAssigningSameFields,
   Debt = (2 + 5*fieldsAssigned.Count() + 5*otherMethodsAssigningSameFields.Count() ).ToMinutes().ToDebt(),
   Severity = Severity.High
}

//<Expl>
// The *{0}* is assigning the *{2}*.
// From the caller perspective, it's generally unexpected and 
// counterintuitive for a property's getter to have *side-effect*s
//</Expl>

//<Desc>
// A property getter with *side-effects* is unexpected and counterintuitive
// for callers: getters should be *pure*, assigning no field.
//
// To avoid matching *lazy initialization at first access* (an invisible
// implementation detail for clients), getters assigning a field only
// assigned by themselves and the corresponding setter are not matched, nor
// are fields with a name similar to the property name.
//</Desc>

//<HowToFix>
// Make matched property getters assign no field.
//
// Estimated Debt: 2 minutes plus 5 minutes per field assigned and 5 minutes
// per other method assigning such field.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Immutability: all 15 items