realvizu/NsDepCop

Method call dependencies without a return type are ignored

kirchsth opened this issue · 1 comments

Hi @realvizu

If a static method is called the dependency itself is ignored, only the return type is checked.
E.g.

  • if ClassB calls ClassA ClassA.IsCalledReturnsClassA() then it detects the method dependency based on the return type;
  • if ClassB calls void IsCalledReturnsVoid() then the method dependency is ignored;

You can reproduce it with following files:
source.cs

namespace B
{
    using static A.ClassA;

    public class ClassB
    {
        public void CallA()
        {
            IsCalledReturnsVoid();    // call dependency itself is ignored
            IsCalledReturnsClassA();  // produces warning based on return type (but not on the call itself)
        }
    }
}

namespace A
{
    public class ClassA
    {
        public static void IsCalledReturnsVoid() { _ = new ClassA();}
        public static ClassA IsCalledReturnsClassA() { return new ClassA(); }
    }
}

config.nsdepcop

<?xml version="1.0" encoding="utf-8"?>
<NsDepCopConfig>
    <Allowed From="*" To="*" /> 
    <Disallowed From="B.*" To="A.*" />
</NsDepCopConfig>

displayed warnings
grafik

I tried to fix it, but I couldn't find it. Do you have a hint for me?

Thank you and BR Helmut

Indeed it seems like a bug, thanks for reporting it.
I guess the effect of 'using static' on the analysis logic was never really looked into.

It should be fixed in SyntaxNodeAnalyzer.DetermineReferencedType but it may not be easy.
When we check whether symbolInfo.Symbol is IMethodSymbol we should consider methodSymbol.ContainingSymbol too, I guess. I advise starting with a test (created in NsDepCop.SourceTest) that reproduces the problem and then using the debugger to look whether methodSymbol.ContainingSymbol is really ClassA in this case and if so then changing DetermineReferencedType to return a collection instead of a single ITypeSymbol. Unfortunately it can have a ripple effect.