Roslyn-analyzers: CA1303 fires on nameof(caption)

[gewarren]

Analyzer package

Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

v2.9.6

Diagnostic ID

Example: CA1303

Repro steps

Add the following C# code to a project:

public class Helpers
{
    [Localizable(true)]
    public string Text { get; set; }

    public Helpers(string caption)
    {
        Text = nameof(caption);
    }
}

Expected behavior

CA1303 doesn't fire.

Actual behavior

CA1303 fire on nameof(caption).

[Evangelink]

@mavasani This doesn't look like an error to me. The property Text is marked as localizable and we set non-localizable value to it.

[mxashlynn]

I am wondering about a slightly different case.

In this example it is the name of the argument defined by ArgumentException rather than the value of an attribute that causes warning CA1303.

using System;
namespace ExampleCode
{
    class Program
    {
        public void ReportMyError(int myVariable)
        {
            throw new Exception($"There was a bad value in {nameof(myVariable)}.");
        }

        static void Main(string[] args) { }
    }
}

Is this also the intended behavior?
If so, how ought I to report the names of variables in diagnostic messages such as this?

Should I be passing the string with the embedded nameof to ResourceManager.GetString?

Thank you!

[Evangelink]

There is something weird going on but that doesn't seem to be linked to the nameof

[Fact]
public async Task NewException()
{
    await VerifyCS.VerifyAnalyzerAsync(@"
using System;
class MyClass
{
    public void ReportMyError(int myVariable)
    {
        switch (myVariable)
        {
            case 0:
                throw new Exception(""text.""); // warning here

            case 1:
                throw new Exception($""text.""); // warning here

            case 2:
                throw new Exception($""text: {myVariable}.""); // no warning here

            case 3:
                throw new Exception($""text: {nameof(myVariable)}.""); // warning here
        }
    }
}");
}

[Evangelink]

@mxashlynn The rule expects all parameters named message, text or caption to be localized (https://github.com/dotnet/roslyn-analyzers/blob/master/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/DoNotPassLiteralsAsLocalizedParameters.cs#L285-L287) so in your case you shall be calling:

throw new Exception(string.Format(CultureInfo.CurrentCulture, MyResx.MyEntry, nameof(myVariable)));

@mavasani maybe the rule needs some tweaking because we do not catch correctly the case case 2 as the stringContentValue.IsLiteralState returns false. To be honest, I am not really quite sure to understand why we are going for a cfg build for this rule anyways.

[mxashlynn]

Thank you very much! I appreciate you pointing me in the right direction.

[Evangelink]

@mxashlynn BTW if you don't want to have exception messages translated, you shouldn't feel forced to.

[mxashlynn]

Thanks, that makes sense. For the moment I'm aiming to learn .NET best practices, but in general I would also want to balance how much is gained by following a given rule vs the work it takes to implement and maintain it.

[MaxDZ8]

I am a bit concerned the syntax to circumnavigate the issue seems a lot of work. It's possibly better to add a disable annotation.
As it stands now code such as:

public class Gizmo
{
    public IReadOnlyList<byte> Blobby{ get; }

    public Gizmo(List<byte> blobby)
    {
        if (null == blobby) throw new NullReferenceException(nameof(blobby));
        Blobby = blobby;
    }
}

Triggers error. Considering there's a rule about using nameof I am surprised there's no exception (as in implicit exclusion from localization rule) in this case as it seems fairly common.

Are there any other options?

For the time being, I have globally disabled the lint by global suppression. It seems to me it is the lesser evil.

Edit: the thing is ok with ArgumentNullException, the point is therefore sorta moot. I'm not completely sure the thing works in general but I'm still leaving it for future readers.

[mavasani]

I believe this issue no longer repros with latest bits. Can you try this package: https://dotnet.myget.org/feed/roslyn-analyzers/package/nuget/Microsoft.CodeAnalysis.FxCopAnalyzers/3.3.0-beta1.20223.1+3ab544e7?

[MaxDZ8]

Confirmed; it seems to be fixed!

[Evangelink]

@mavasani I believe this ticket can be closed.