Add warning when property has a backing field but a manually implemented accessor does not use it

[RikkiGibson]

#57012

The goal of this change is to catch a scenario where user is in an in-between state of using the field keyword or not. See MigratingToFieldKeyword_ tests.

This is designed to work as a basically declaration-level check. The goal was for it to be cheap and for diagnostics to appear relatively early without needing to e.g. compile the whole property and constructors, and analyze definite assignment, in order for them to appear. (i.e. a mini version of what happens at the compilation-level for ordinary fields.)

It's not meant to be airtight, but to just catch simple cases. For example, it won't complain if the keyword is used in all accessors, yet none of the uses are writing to it.

The design has a slightly odd rule as well relating to initializers. Basically, presence of an initializer removes the need for setter to use the backing field. This is because something is certainly setting the backing field if an initializer is present. This scenario was suggested by @jnm2, and we think would come up in a situation where an interface property is "read/write" but the implementation is readonly. See MigratingToFieldKeyword_05.

[RikkiGibson]

@cston @333fred for review

[cston]

                public string Prop { get; set => throw null!; } // 1

Minor: The ! in this test shouldn't be necessary.

---

Refers to: src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs:347 in b6903fd. [](commit_id = b6903fd, deletion_comment = False)

[cston]

                partial object P1

Consider testing declarations appearing after implementations as well, in this test and the next test. (See existing PartialProperty_* tests that use bool reverseOrder.)

---

Refers to: src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs:264 in b6903fd. [](commit_id = b6903fd, deletion_comment = False)

[cston]

            //     public string Prop { get; set => throw null!; } // 1

The warning for this case seems unfortunate.

---

Refers to: src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs:363 in b6903fd. [](commit_id = b6903fd, deletion_comment = False)

[333fred]

Consider inlining this local function.

[333fred]

Honestly, the backing 'field' reads very awkwardly. Maybe just should use 'field'?

[RikkiGibson]

            //     public string Prop { get; set => throw null!; } // 1

The warning for this case seems unfortunate.

Refers to: src/Compilers/CSharp/Test/Emit3/FieldKeywordTests.cs:363 in b6903fd. [](commit_id = b6903fd, deletion_comment = False)

I think this case itself, I am fine with. We can actually see that nothing is setting that field--why are you using a field? You can use a property initializer to insist that actually it is fine, if you really want.

One I am less sure about is string Prop { get => field ??= Compute(); set => throw null!; }. In this case an initializer actually needs to be added to silence the warning, which the diagnostic message isn't clear about currently.

At the same time I am reluctant to abandon this syntax/declaration-based approach, and do a definite assignment pass instead a la ordinary fields, because for the getter-only case we would potentially need to analyze the constructors to know if the field is being set.

[jnm2]

My personal preference would be for airtightness, and the lack of earliness (having to wait for a full compilation to update the warnings) hasn't been a noticeable problem for me with existing fields and field-like events.

[cston]

It looks like the CI build failures are due to the fix in #75461.

[RikkiGibson]

Have addressed feedback, merged in main, and updated more test baselines.