JetBrains / resharper-unity

Unity support for both ReSharper and Rider
Apache License 2.0
1.21k stars 134 forks source link

[RIDER] - incorrect behavior of "convert to auto property" #1017

Closed rob8861 closed 4 years ago

rob8861 commented 5 years ago

Consider the following

public int X => _x;

[SerializeField] private int _x;

Rider will suggest to convert the property X to an auto property, and when it does that it also adds [field: SerializedField] which causes some unwanted behaviors. I think this behavior should be suppressed when a property is pointing to a [SerializedField] field.

image

rob8861 commented 5 years ago

@citizenmatt Hi Matt, Can someone respond whether this is a bug / by design/ etc?

it's been 15 days since I reported it. Should I post bugs in YouTrack moving forward? Thanks,

citizenmatt commented 5 years ago

No, things can still be reported here. It's the same people who'd see them in either place. The underlying R# behaviour is fine, but does have unfortunate side effects for Unity code. We'll look at suppressing it.

rob8861 commented 5 years ago

thank you.

KWaldt commented 5 years ago

If I understand correctly, this is still being worked on. I just wanted to add another example:

The suggestion (auto-property) is especially strange when you only have a get or set in your property. Rider suggest replacing this: [SerializeField] private string usageHotkey; public string UsageHotkey{ get => usageHotkey;} with this: [field: SerializeField] public string UsageHotkey { get; } This looks less pretty in the editor, and Rider throws another note, saying that the get-only property is never assigned. image

The original code doesn't throw a warning/suggestion if you replace private with protected. Basically, I just want to be able to set my value in the inspector (ScriptableObject), but allow the public to only read it. It works just fine, but the suggestion is distracting in this case (and supressing it with comments creates more unnecessary lines). I don't want to disable it, though, since it's normally useful.

somethingSTRANGE commented 5 years ago

Rider suggest replacing this: [SerializeField] private string usageHotkey; public string UsageHotkey{ get => usageHotkey;} with this: [field: SerializeField] public string UsageHotkey { get; } This looks less pretty in the editor, and Rider throws another note, saying that the get-only property is never assigned. image

I would also like to point out that while the backing field name associated with an auto-property without a setter is shown in the Inspector in "normal" mode, it doesn't appear when the Inspector is in "debug" mode. Adding a private setter to the auto-property will allow the backing field to appear in the "debug" mode Inspector as well.

krasnotsvetov commented 4 years ago

The issue is fixed in upcoming 19.3 release

Zaimatsu commented 3 years ago

I'm using this notation in my code: [field: SerializeField] public string UsageHotkey { get; private set; } And I get Auto-property accessor is never used warning.

When I change this to get only property: [field: SerializeField] public string UsageHotkey { get; } I get Get-only auto-property 'UsageHotkey' is never assigned warning.

Is it a bug or can I somehow cofigure Rider to detect that this auto-property is set by inspector?

Edit: Also it's worth to mention that I'm using Unity 2020.1.14f1 and it's working just fine with auto-properties in inspector.

citizenmatt commented 3 years ago

Thanks @Zaimatsu, there isn't a way to tell Rider to automatically detect this issue, although you can use the Alt+Enter menu to add a comment to the code to suppress the warning. I've added a new issue to track updating Rider to automatically suppress the warning: #1949

noio commented 5 months ago

I don't think I'm understanding the solution here.

I want rider to "convert to auto-property" in general, but not when it is a Unity [SerializeField] or [SerializeReference], because that changes the serialization name, and will lose serialized data!

Is there a way to turn off only the latter?