JetBrains / resharper-unity

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

Suggestion to rewrite as ??= is incorrect #1818

Open aybe opened 4 years ago

aybe commented 4 years ago

(Visual Studio)

In the following code:

using UnityEngine;

public class Billboard : MonoBehaviour
{
    public Camera Camera;

    private void Start()
    {
        if (Camera is null)
        {
            Camera = Camera.main;
        }
    }

    private void Update()
    {
        transform.LookAt(Camera.transform.forward);
    }

    private void OnDrawGizmos()
    {
        Debug.DrawRay(transform.position, transform.forward);
    }
}

It suggests to rewrite as Camera ??= Camera.main which won't work.

But if you'd write this, the suggestion doesn't show which is correct:

        if (Camera == null)
        {
            Camera = Camera.main;
        }
aybe commented 4 years ago

It turns out that actually the is syntax isn't even valid for Unity !

"null" object won't be considered as being null.

So, Resharper should instead warn you that such syntax won't produce the expected behavior.

citizenmatt commented 4 years ago

Can you check the language level for the project, please? Right click on the project in Solution Explorer and select Properties. You should see language level in there. It looks like R# is recommending C# 8 features, even though Unity doesn't (officially) support C# 8 yet.

You can also take a look at the contents of the .csproj file, and look for LangVersion. If you're using ReSharper, the project file is generated by the Visual Studio Unity integration, and I'm not sure if they set the language version to a specific supported version (e.g. 7.3) or to a default value such as latest.

Also make sure to install the Unity plugin for ReSharper. This will override ReSharper's view of the language level to something that Unity can handle.

aybe commented 4 years ago

I have enabled C# 8.0 (Unity 2020 alpha) and have the plugin installed.

citizenmatt commented 4 years ago

OK. Looks like we need to suppress the ??= suggestion. I think I might have misread the comment about is null. Do you mean that this doesn't work at all in Unity, or doesn't work as you'd expect? I.e. it checks if the managed object is null, but doesn't call the custom equality operator so doesn't check the status of the native game object?

aybe commented 4 years ago

Exactly, the is operator doesn't return the same result as == does. And since this is a keyword whose behavior can't be overriden, it's probably a good idea that R# warns you about the situation. Le 28 août 2020 10:12, Matt Ellis notifications@github.com a écrit : OK. Looks like we need to suppress the ??= suggestion. I think I might have misread the comment about is null. Do you mean that this doesn't work at all in Unity, or doesn't work as you'd expect? I.e. it checks if the managed object is null, but doesn't call the custom equality operator so doesn't check the status of the native game object?

—You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.