DotNetAnalyzers / StyleCopAnalyzers

An implementation of StyleCop rules using the .NET Compiler Platform
MIT License
2.65k stars 508 forks source link

SA1313: Parameter '__' must begin with lower-case letter. #2974

Open glennawatson opened 5 years ago

glennawatson commented 5 years ago

I'm aware of #1606 and noticed in that issue it says that it should be fixed for 1.1.118. On my project I encourage people to create new issues for old bugs so doing the same here.

This seems to be happening in 1.1.118

So an example where it is happening is

            var viewLoaded = Observable.FromEvent<RoutedEventHandler, bool>(
                eventHandler =>
                {
                    void Handler(object _, RoutedEventArgs __) => eventHandler(true);
                    return Handler;
                },
                x => fe.Loading += x,
                x => fe.Loading -= x);

This is using a dependency on System.Reactive and it's event observable converters.

glennawatson commented 5 years ago

Attached is a small sample repo with the problem StylecopExample.zip

sharwell commented 5 years ago

📝 It's possible this is related to the use of local functions.

StepKie commented 5 years ago

Same for me, for the single-underscore discard. (using 1.1.118)

wiciok commented 3 years ago

@sharwell This issue also happens for me on discarded method parameter:

public class ZXC
    {
        /// <summary>
        /// test.
        /// </summary>
        /// <param name="_"></param>
        /// <returns></returns>
        public Task<IEnumerable<Bar>> HandleAsync(Foo _)
        {
            throw new System.NotImplementedException();
        }
    }

No use of local functions at all.

Version 1.2.0.261 (Unstable)

sharwell commented 3 years ago

discarded method parameter

This case is covered by #2599

Method parameters cannot be discards. The recommended approach in this case is to give the parameter a proper name, and then assign it to a discard on the first line:


public Task<IEnumerable<Bar>> HandleAsync(Foo value)
{
  _ = value;

  throw new System.NotImplementedException();
}
sharwell commented 3 years ago

💡 Note that C# 9 now supports multiple explicit discards for lambdas.

DavidMartynWood commented 2 years ago

Since the discussion on #2599 was closed as won't fix. What are we doing with this one? I was going to fix it but I see we are explicitly only allowing this for lambdas? Should we allow it for local functions too? Or should we be consistent and only allow it for lambdas?

Current behaviour

Not allowed

public void MethodName(int __)
{
}

Not allowed

public void TestMethod()
{
    void Handler(object _, EventArgs __)
    {
    }

    EventHandler handler = Handler;
    handler(this, EventArgs.Empty);
}

Allowed

public void MethodName()
{
    System.Func<int, int> function1 = __ => __;
    System.Func<int, int> function2 = (__) => __;
    System.Func<int, int> function3 = delegate(int __) { return __; };
}
DemetriouJohn commented 2 years ago

@DavidMartynWood if the whole point is to use discards, why not use single underscore which is also understood by the compiler? Am I missing something here?

bjornhellander commented 9 months ago

This is tagged bug and up for grabs, is that correct? Personally, I would rather like to see local functions treated just like normal methods, i.e. requiring proper parameter names, and allowing _ and __ only in lambdas. That would be just like it is implemented today, if I am not mistaken. Do you want to allow underscore parameters in local functions, @sharwell?

DavidMartynWood commented 8 months ago

@DavidMartynWood if the whole point is to use discards, why not use single underscore which is also understood by the compiler? Am I missing something here?

I do not know, I just commented on this ticket to see if it was an easy fix for myself to implement. It does seem to have limited use cases so maybe we should close it.

azarasi3 commented 3 weeks ago

Nice to meet you, @sharwell

💡 Note that C# 9 now supports multiple explicit discards for lambdas.

In #2645, we allowed __ but __ becomes SA1313. Considering the readability of ,

System.Action<int> action = __ => { };

should be

System.Action<int> action = _ => { };

but the fact that a single __ is allowed is contradictory.

The issue with lambda expression parameter names can be resolved by apply the following three options:

What do you think?

azarasi3 commented 2 weeks ago

Although it was decided not to fix it in #2599, since there is an exception rule for in SA1313, we think that it can be resolved by allowing parameters starting with as an exception when they are unused, in response to IDE0060 in SA1313.