S1144: report implemented interface (affects also S2325)

[git-ruttmann]

Description

The implementation of an interface is mocked by a private class.
Only parts of the interface are tested in this context. The other parts are not accessed and throw a NotImplementedException.

Rule S1144 tells that the implementation is not used.
Rule S2325 tells to make the method / property static, but thats not possible for interface elements.

Repro steps

Create an interface

public interface IMockMe
{
  int TestData { get; }
  int OtherData { get; }
}

Implement that class as private class in a test

[TestClass]
public class TestWithMock
{
  [TestMethod]
  public void TestOne()
  {
    var testTarget = ...;
    var input = new TestMockData(11);
    IMockMe transformedData = testTarget.TransformData(input);
    Assert.AreEqual(15, transformedData.Data);
  }

  private class TestMockData : IMockMe
  {
    public TestMockData(int testData)
    {
      this.TestData = testData;
    }

    public int TestData { get; }
    public int OtherData => throw new NotImplementedException();
  }
}

Expected behavior

Rule S1144 should not report an interface element as unused. Interface Elements that are never used in any implementation should be reported at interface level (I guess, that is the case already).

Rule S2325 should not trigger for interface elements as they cannot be static.

Actual behavior

Rule S1144 reports interface elements as unused but they must be implemented.
Rule S2325 reports interface elements as "make it static" but interface elements can't be static.

Known workarounds

None

Related information

Unluckily the version is not known, Sonar runs as current version of Codacy.com as of 17.7.2020.

[pavel-mikula-sonarsource]

Hi @git-ruttmann ,

I'm unfortunately unable to reproduce the issue with SonarLint 4.23 and therefore analyzer for C# 8.9. Can you please try to install the extension and re-validate the issue on your side to confirm it's and old version issue on Codacy.com side? It will also bring the code quality directly into your Visual Studio.

If you're not using Visual Studio, you can try to embed our analyzer as NuGet package just to verify it.

[git-ruttmann]

Hi Pavel,

I've tried it with the visual studio plugin and it was not reported.

The next step i've tried was a local unittest in the UnusedPrivateMemberTest. The interface definition is in the same "file". The false positive was not reported.

Here is the code:

        [TestMethod]
        [TestCategory("Rule")]
        public void UnusedPrivateMember_PrivateInterfaceImplementation()
        {
            Verifier.VerifyCSharpAnalyzer(@"
using System;
public class PropertyUsages
{
    public interface IData
    {
        int Used { get; }
        int OtherUse { get; }
    }

    public interface IWorker
    {
        IData Process(IData source);
    }

    public class TestDataProcessing
    {
        public void ProcessMockedData(IWorker worker)
        {
            var mockedInput = new MockedData { Used = 7 };
            var result = worker.Process(mockedInput);
        }

        private class MockedData : IData
        {
            public int Used { get; set; }
            public int OtherUse => throw new NotSupportedException();
        }
    }
}
", new CS.UnusedPrivateMember());
        }

So I've done some further investigations and extracted the analyzer call from codacy-sonar-csharp. The analysis is run file by file
and creates a new workspace for each file. The false positive was reproducible with sonar-dotnet version 8.10.0.139, if the interface definition is in a separate file.

But the error is not reported, if I add both, the interface file and the test file, to the same ad-hoc workspace. Looks like the error is only reported for undefined interfaces, which makes sense to me.
Actually I have no Idea, why the compilation succeeds with only a single file. If I remove the interface definition in the unit test, the compilation fails.

IMHO this is the problem of codacy, not of sonar-dotnet, so I'd vote to close the issue.

[pavel-mikula-sonarsource]

Hi @git-ruttmann ,

That's a great investigation! And I confirm it's a Codacy issue.

The compilation should always succeed. Even for malformed syntax, missing references and so on. Some semi-random syntax tree is produced. And the compilation object will contain reported errors. It's up to Codacy to handle or ignore those. You can take a look at this for curiosity.

Like this, Codacy doesn't have to compile the project and they save themselves issues with compilation configuration, but they can only detect some easy code quality issues that don't need any semantic information from outer files or references.