Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IParameterReferenceExpression operation not generated for parameter access in a certain cases #8884

Closed
mavasani opened this issue Feb 18, 2016 · 9 comments · Fixed by #18976 or #19307
Assignees
Labels
Area-Analyzers Bug Feature - IOperation IOperation Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Urgency-Soon
Milestone

Comments

@mavasani
Copy link
Contributor

Sample test code:

using System;

public class NeatCode
{
    // Used parameter methods
    public void UsedParameterMethod1(Action a)
    {
        Action a2 = new Action(() =>
        {
            a(); // No parameter reference expression operation generated for this access
        });
    }
}

This does not repro for VB.

@mavasani
Copy link
Contributor Author

Quick repro: Build roslyn-analyzers into RoslynDev hive and create a project with above code - you should see CA1801 (remove unused parameter) warning.

@mavasani mavasani added the Feature - IOperation IOperation label Feb 18, 2016
@mavasani mavasani changed the title IParameterReferenceExpression operation not generated for parameter access in a lambda IParameterReferenceExpression operation not generated for parameter access in a certain cases Feb 18, 2016
@mavasani
Copy link
Contributor Author

Found couple more cases where this repros:

  1. Parameter access in a using statement (repros for both VB and C#)
  2. Parameter access in a linq statement (repros for both VB and C#)
using System;
using System.Reflection;

class C
{
    void F(int x, IDisposable o)
    {
        using (o)
        {
            int y = x; // no parameter reference received for 'x'
        }
    }

    private object F2(Assembly assembly)
    {
        return (from t in assembly.GetTypes()
                    select t.Attributes).FirstOrDefault();  // no parameter reference received for 'assembly'
    }
}

@mavasani mavasani added this to the 1.2 milestone Feb 19, 2016
mavasani added a commit to mavasani/roslyn-analyzers that referenced this issue Feb 19, 2016
mavasani added a commit to dotnet/roslyn-analyzers that referenced this issue Feb 19, 2016
@msJohnHamby msJohnHamby modified the milestones: 1.3, 1.2 Feb 24, 2016
@mavasani
Copy link
Contributor Author

This is causing lot of false reports in Analyzers.sln in roslyn-analyzers repo - we should probably test the fix for this bug against that solution.

@mavasani
Copy link
Contributor Author

We get no parameter reference for parameter type if used in a string as follows: $"{type.Name}"

@bkoelman
Copy link
Contributor

bkoelman commented May 5, 2016

Yes, I also get false reports for CA1801 in interpolated strings and LINQ queries.

Additionally, I see unexpected reports on parameter usage in initializers:

        private Panel CreateRankingsOverlay(string text)
        {
            return new Panel
            {
                Controls =
                {
                    new AutoScalingLabel
                    {
                        Text = text,
                    }
                }
            };
        }
            private static char AutoDetectFieldSeparator(string line)
            {
                var charCounts = new Dictionary<char, int>
                {
                    { '\t', line.Count(c => c == '\t') },
                    { ';', line.Count(c => c == ';') },
                    { ',', line.Count(c => c == ',') },
                    { ':', line.Count(c => c == ':') },
                    { '|', line.Count(c => c == '|') }
                };

                int highestOccurrence = charCounts.Max(pair => pair.Value);
                return charCounts.First(pair => pair.Value == highestOccurrence).Key;
            }

@mavasani
Copy link
Contributor Author

@cston found another case for pointer expressions:

unsafe class C
{
    static int F(byte* b, int x, int y)
    {
        return b[x] + y;
    }
}

No parameter reference generated for b and x.

sharwell added a commit to sharwell/roslyn-analyzers that referenced this issue Apr 7, 2017
mavasani pushed a commit to dotnet/roslyn-analyzers that referenced this issue Apr 10, 2017
* Disable "Review Unused Parameters" analyzer

See dotnet/roslyn#8884

* Remove suppressions for CA1801 false positives
@mavasani mavasani self-assigned this Apr 17, 2017
mavasani added a commit to mavasani/roslyn that referenced this issue Apr 25, 2017
… nodes for whom IOperation support is not yet implemented

This ensures that the analyzer driver/operation walker is able to find all the descendant IOperation nodes within these not yet implemented features. We should remove the IOperationWithChildren interface once we have designed all the IOperation APIs.

TODO: Implement the VB part.

Fixes dotnet#8884
@mavasani
Copy link
Contributor Author

mavasani commented May 5, 2017

Re-activating as we still have couple of cases which have not been addressed with #18976. More specifically, parameter references in delegate creation and collection initializers.

@mavasani mavasani reopened this May 5, 2017
mavasani added a commit to mavasani/roslyn that referenced this issue May 5, 2017
…ssion

The current IOperation API implementation of BoundDelegateCreationExpression has a bunch of issues and needs to be redesigned. This is tracked by dotnet#8897 and will be addressed post 15.3. Meanwhile, to unblock analyzers on code containing delegate creation expressions with lambda arguments, this bound node has been switched to OperationKind.None with override for Children property.

Fixes the first repro case provided in dotnet#8884
@jinujoseph jinujoseph added the 4 - In Review A fix for the issue is submitted for review. label May 5, 2017
@sharwell sharwell added Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed 4 - In Review A fix for the issue is submitted for review. labels May 26, 2017
@AlekseyTs AlekseyTs reopened this Jun 5, 2018
@AlekseyTs AlekseyTs removed the Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented label Jun 5, 2018
@AlekseyTs
Copy link
Contributor

It looks like the problem still exists in some scenarios:


        <CompilerTrait(CompilerFeature.IOperation)>
        <Fact, WorkItem(8884, "https://github.com/dotnet/roslyn/issues/8884")>
        Public Sub ParameterReference_NoPiaObjectCreation()
            Dim sources0 = <compilation>
                               <file name="a.vb"><![CDATA[
Imports System.Runtime.InteropServices
<Assembly: ImportedFromTypeLib("_.dll")>
<Assembly: Guid("f9c2d51d-4f44-45f0-9eda-c9d599b58257")>
<ComImport()>
<Guid("f9c2d51d-4f44-45f0-9eda-c9d599b58277")>
<CoClass(GetType(C))>
Public Interface I
    Property P As Integer
End Interface
<Guid("f9c2d51d-4f44-45f0-9eda-c9d599b58278")>
Public Class C
    Public Sub New(o As Object)
    End Sub
End Class
]]></file>
                           </compilation>
            Dim sources1 = <compilation>
                               <file name="a.vb"><![CDATA[
Structure S
    Function F(x as Object) As I
        Return New I(x)'BIND:"New I(x)"
    End Function
End Structure
]]></file>
                           </compilation>
            Dim compilation0 = CreateCompilationWithMscorlib40(sources0)
            compilation0.AssertTheseDiagnostics()

            ' No errors for /r:_.dll
            Dim compilation1 = CreateEmptyCompilationWithReferences(
                sources1,
                references:={MscorlibRef, SystemRef, compilation0.EmitToImageReference(embedInteropTypes:=True)})

            Dim expectedOperationTree = <![CDATA[
INoPiaObjectCreationOperation (OperationKind.None, Type: I, IsInvalid) (Syntax: 'New I(x)')
  Initializer: 
    null
]]>.Value

            Dim expectedDiagnostics = <![CDATA[
BC30516: Overload resolution failed because no accessible 'New' accepts this number of arguments.
        Return New I(x)'BIND:"New I(x)"
               ~~~~~~~~
]]>.Value

            VerifyOperationTreeAndDiagnosticsForTest(Of ObjectCreationExpressionSyntax)(compilation1, "a.vb", expectedOperationTree, expectedDiagnostics)
        End Sub

@AlekseyTs
Copy link
Contributor

And for C# as well:


        [CompilerTrait(CompilerFeature.IOperation)]
        [Fact, WorkItem(8884, "https://github.com/dotnet/roslyn/issues/8884")]
        public void ParameterReference_NoPiaObjectCreation()
        {
            var sources0 = @"
using System;
using System.Runtime.InteropServices;

[assembly: ImportedFromTypeLib(""_.dll"")]
[assembly: Guid(""f9c2d51d-4f44-45f0-9eda-c9d599b58257"")]
[ComImport()]
[Guid(""f9c2d51d-4f44-45f0-9eda-c9d599b58277"")]
[CoClass(typeof(C))]
public interface I
        {
            int P { get; set; }
        }
[Guid(""f9c2d51d-4f44-45f0-9eda-c9d599b58278"")]
public class C
{
    public C(object o)
    {
    }
}
";
            var sources1 = @"
struct S
{
	public I F(object x)
	{
		return /*<bind>*/new I(x)/*</bind>*/;
    }
}
";
            var compilation0 = CreateCompilation(sources0);
            compilation0.VerifyDiagnostics();

            var compilation1 = CreateEmptyCompilation(
                sources1,
                references: new[] { MscorlibRef, SystemRef, compilation0.EmitToImageReference(embedInteropTypes: true) });

            string expectedOperationTree = @"
INoPiaObjectCreationOperation (OperationKind.None, Type: I, IsInvalid) (Syntax: 'new I(x)')
  Initializer: 
    null
";
            var expectedDiagnostics = new DiagnosticDescription[] {
                    // (6,25): error CS1729: 'I' does not contain a constructor that takes 1 arguments
                    // 		return /*<bind>*/new I(x)/*</bind>*/;
                    Diagnostic(ErrorCode.ERR_BadCtorArgCount, "(x)").WithArguments("I", "1").WithLocation(6, 25)
            };

            VerifyOperationTreeAndDiagnosticsForTest<ObjectCreationExpressionSyntax>(compilation1, expectedOperationTree, expectedDiagnostics);
        }

@jinujoseph jinujoseph added the Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented label Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Analyzers Bug Feature - IOperation IOperation Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Urgency-Soon
Projects
None yet
8 participants