DateOnly.AddX methods should be marked [Pure]

[hughesjs]

Description

The methods in System.DateOnly such as .AddYears(int) are pure in that they create a new instance of DateOnly with the modified values.

However, they haven't been decorated with the [Pure] attribute which means that IDEs (and users by extension) might not be aware of this and might be expecting it to mutate the variable itself. IIRC, most IDEs will generate a warning if you don't capture the value of a [Pure] method call.

Tbh, this probably ought to be a compiler warning as well.

Reproduction Steps

var x = DateOnly.MinValue;
// These lines should produce a warning about not capturing the value of a pure method
x.AddYears(1);
x.AddDays(1);
x.AddMonths(1);

Expected behavior

At the very least, IDEs that are aware of the [Pure] attribute (I believe as a minimum VS and Rider both are) should display a squiggly about not capturing the value of a pure method.

Ideally, the compiler should also generate a warning.

Actual behavior

No warnings are created as the methods aren't declared as pure.

Regression?

No

Known Workarounds

No response

Configuration

Version: .NET 6.0
OS: Mac OS 11.6.1
Arch: x64
Config Specific: No

Other information

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[KalleOlaviNiemitalo]

Some previous requests to add [Pure] elsewhere were rejected. See #24659, #33414 (comment), #34098 (comment).

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

The methods in System.DateOnly such as .AddYears(int) are pure in that they create a new instance of DateOnly with the modified values.

However, they haven't been decorated with the [Pure] attribute which means that IDEs (and users by extension) might not be aware of this and might be expecting it to mutate the variable itself. IIRC, most IDEs will generate a warning if you don't capture the value of a [Pure] method call.

Tbh, this probably ought to be a compiler warning as well.

Reproduction Steps

var x = DateOnly.MinValue;
// These lines should produce a warning about not capturing the value of a pure method
x.AddYears(1);
x.AddDays(1);
x.AddMonths(1);

Expected behavior

At the very least, IDEs that are aware of the [Pure] attribute (I believe as a minimum VS and Rider both are) should display a squiggly about not capturing the value of a pure method.

Ideally, the compiler should also generate a warning.

Actual behavior

No warnings are created as the methods aren't declared as pure.

Regression?

No

Known Workarounds

No response

Configuration

Version: .NET 6.0
OS: Mac OS 11.6.1
Arch: x64
Config Specific: No

Other information

No response

Author:	hughesjs
Assignees:	-
Labels:	area-System.Runtime, untriaged
Milestone:	-

[tannergooding]

IDEs and tooling that only consider the Pure attribute are "outdated" in my opinion.

The Pure attribute has never been actually enforced and never had "significant" usage throughout the BCL or other key foundations of the ecosystem.

Modern C#/.NET have newer concepts such as readonly struct and readonly members that do at least get language validation and enforcement (ignoring unsafe code) and the "purity" of something like DateTime.AddX is implied by it being an instance member of a readonly struct. The same goes for methods like Vector4.CopyTo, where Vector4 isn't readonly but the CopyTo method is and that indicates that the method cannot mutate instance state (and this is validated by the language for safe code).

The concept from "pure" that is missing is for static/instance methods mutating static state, but due to back-compat among other things I would expect that the existing Pure attribute couldn't be reused or enforced. It would end up being some new language enforced attribute if that was ever introduced.

[stephentoub]

Yes, as I outlined in #34098 (comment), I'd be happy to see us introduce a [DoNotIgnoreResult] attribute and associated analyzer, but that's distinct from [Pure]. I don't want to further propagate the use of [Pure].

[tannergooding]

Modern .NET likewise provides two built-in analyzers covering "you forgot to use the value":

• https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0058
• https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0059

These cover the scenario of "you called a method that returns a value, but aren't consuming the value anywhere".