Refactor IsAutoPropertyWithGetAccessor usage in MethodCompiler

[Youssef1313]

@AlekseyTs This might not be what you're expecting. It's not much clear to me what "clean up" is needed.

Test plan: #57012
Proposal: dotnet/csharplang#140

[AlekseyTs]

Done with review pass (commit 2)

[Youssef1313]

@AlekseyTs I reverted all the simple inlinings for now.

[AlekseyTs]

        this.CheckModifiers(location, hasBody: true, isAutoPropertyOrExpressionBodied: true, diagnostics: diagnostics);

It looks like we can remove this parameter completely, hasBody should be sufficient.

---

In reply to: 1002657565

---

Refers to: src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs:188 in 6d53a72. [](commit_id = 6d53a72, deletion_comment = False)

[AlekseyTs]

        bool hasBody,

Would it make sense to rename this to "hasBlockBody"?

---

In reply to: 1002661648

---

Refers to: src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs:198 in 6d53a72. [](commit_id = 6d53a72, deletion_comment = False)

[AlekseyTs]

Done with review pass (commit 9)

[AlekseyTs]

LGTM (commit 12)

[AlekseyTs]

@333fred, @dotnet/roslyn-compiler For the second review.

[Youssef1313]

@AlekseyTs Is there any more refactoring needed before continuing working on the feature?

If not, then the next step is looking at supporting mixed scenarios right? (#57076 (comment) for reference)

[AlekseyTs]

Is there any more refactoring needed before continuing working on the feature?

I think there is. We need to examine other usages of "AutoProperty" related APIs and introduce suitable abstractions that can be used instead and were not about the "AutoProperty" term. I gave you an example here #58515 (comment). The theory there is probably not quite accurate, but it should be possible to get the idea. Let's start with that specific place and go from there.

Also, before we start working on any behavior changes, I would like us to get earlier identified tests in place. In particular those about SemanticModel and GetMembers API.

[AlekseyTs]

The link to the comment doesn't quite work. It was about a change in earlier iteration of this PR in AccessingAutoPropertyFromConstructor in BinderStatements.cs

Here is the interesting part:

Without taking a deep look, here is the theory that I have: "Here we actually want to check if an assignment to a property within constructor is supposed to assign to a backing field instead." If my theory is generally correct, let's add a new property with the specific name and purpose, and let's use it here. It would feel appropriate to rename the containing method accordingly as well. This is the process that we should go through for each API usage, one by one.

Again, without significant additional thinking, I have an idea that it might help to add a couple of properties to SourcePropertySymbolBase instead. Something like:

• GetMethodIsEquivalentToBackingFieldRead
• SetMethodIsEquivalentToBackingFieldWrite

Perhaps the AccessingAutoPropertyFromConstructor should become more specific and probably be split between Get and Set access.

Just think about what you can come up with. The goal is to introduce abstractions that do not refer to "AutoProperty" and at the same time likely to have an application for semi-auto properties, or have no relation to them at all.

[Youssef1313]

• GetMethodIsEquivalentToBackingFieldRead
• SetMethodIsEquivalentToBackingFieldWrite

So [Get|Set]MethodIsEquivalentToBackingField[Read|Write] can only be true if the getter/setter is semi-colon only right? It's essentially the same meaning as "Is this a semicolon-only accessor of an auto property", or am I missing something?

[AlekseyTs]

So [Get|Set]MethodIsEquivalentToBackingField[Read|Write] can only be true if the getter/setter is semi-colon only right? It's essentially the same meaning as "Is this a semicolon-only accessor of an auto property", or am I missing something?

Pretty much. However, it would be good to move away from using "an auto property" term even in implementation, when possible. For example, the implementation for properties could be as simple as a check whether BodyShouldBeSynthesized is true for the corresponding accessor, I think.

[AlekseyTs]

@333fred, @dotnet/roslyn-compiler For the second review.

[AlekseyTs]

@333fred, @dotnet/roslyn-compiler For the second review of a community PR.

[333fred]

LGTM (commit 12)