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

Fix S4159: Rule should handle generic interfaces when using System.ComponentModel.Composition #5641

Closed
hymccord opened this issue May 6, 2022 · 3 comments · Fixed by #5883
Assignees
Labels
Area: C# C# rules related issues. Type: False Negative Rule is NOT triggered when it should be. Type: False Positive Rule IS triggered when it shouldn't be.
Milestone

Comments

@hymccord
Copy link

hymccord commented May 6, 2022

Description

This is closely related to #1296, however that fix does not work for this specific MEF attribute type.

The rule raises an FP when implementing a generic interface and exporting it via System.ComponentModel.Composition.

Repro steps

using System.ComponentModel.Composition;

namespace Foo;

class FooOptionsBuilder<T> { }

interface IDbContextOptionsBuilderDecorator<T> 
{
    FooOptionsBuilder<T> Apply(FooOptionsBuilder<T> builder);
}

[Export(typeof(IDbContextOptionsBuilderDecorator<>))]
class WildFooBuilderDecorator<T> : IDbContextOptionsBuilderDecorator<T>
{
    public FooOptionsBuilder<T> Apply(FooOptionsBuilder<T> builder)
    {
        // builder.UseSqlite(); 

        return builder;
    }
}

Expected behavior

The rule doesn't raise an issue.

Actual behavior

An issue is raised.

Known workarounds

This does NOT happen when using ExportAttribute from System.Composition.AttributedModel

Please provide a description of any known workarounds.

Related information

  • dotnet SDK 6.0.202
  • Latest SonarCloud rules used
@hymccord
Copy link
Author

hymccord commented May 6, 2022

Upon further investigation, it seems this may be your intention based on the unit tests. We are seeing this error because we are using MEF attributes, but they are consumed via DryIoc instead of the MEF container since DryIoc does support open generics.

The PR which originally implemented it had a link to an msdn blog from 2009 that said they didn't support open generics but I think things have changed since then.

using MEF1 = System.ComponentModel.Composition;
using System.Composition;
using System.Reflection;
using System.Composition.Hosting;

namespace sq_false_positives;

public static class Program
{
    static void Main(string[] args)
    {
        MEF1();
        MEF2();
    }

    static void MEF1()
    {
        var assemblyCat = new MEF1.Hosting.AssemblyCatalog(Assembly.GetExecutingAssembly());
        var container = new MEF1.Hosting.CompositionContainer(assemblyCat);

        var foo = container.GetExport<FooBuilder<Bar>>().Value;
    }

    static void MEF2()
    {
        var builder = new ContainerConfiguration()
            .WithAssembly(Assembly.GetExecutingAssembly());

        var container = builder.CreateContainer();

        var foo = container.GetExport<FooBuilder<Baz>>();
    }
}


abstract class Foo { }

[Export, MEF1.Export]
class Bar : Foo { }
[Export, MEF1.Export]
class Baz : Foo { }

interface IFooBuilderOptionsDecorator<T> where T : Foo { }

[Export(typeof(IFooBuilderOptionsDecorator<>)), MEF1.Export(typeof(IFooBuilderOptionsDecorator<>))]
class FooBuilderOptionsDecorator<T> : IFooBuilderOptionsDecorator<T> where T: Foo { }

[Export, MEF1.Export]
class FooBuilder<T> where T : Foo
{
    [ImportingConstructor, MEF1.ImportingConstructor]
    public FooBuilder(IFooBuilderOptionsDecorator<T> foo)
    {
        Foo = foo;
    }

    public IFooBuilderOptionsDecorator<T> Foo { get; }
}

I can remove the type constraints and it still works but I'm more showing how we use it.

@costin-zaharia-sonarsource
Copy link
Member

costin-zaharia-sonarsource commented May 24, 2022

Hi @inkahootz, thanks for reporting this.

As far as I can tell there are two different problems:

  • generics are not supported at all -> false positive
  • ExportAttribute from System.Composition.AttributedModel is not supported (it was introduced later, in .Net Core 1.1) -> false negative

We plan to address both topics.

@hymccord
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Type: False Negative Rule is NOT triggered when it should be. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants