beakona / AutoInterface

C# interface-to-member source generator
MIT License
73 stars 9 forks source link

Static analysis attributes #21

Open tecAmoRaller opened 3 months ago

tecAmoRaller commented 3 months ago

Nice generator.

i think, source generator must add static analysis attributes on generated code. https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/attributes/nullable-analysis

For example

interface Serilog.ILogger {
...
    bool BindProperty(string? propertyName, object? value, bool destructureObjects, [NotNullWhen(true)] out LogEventProperty? property)

}

generated file

partial class LoggerWrapper {
    bool Serilog.ILogger.BindProperty(string? propertyName, object? value, bool destructureObjects, out Serilog.Events.LogEventProperty? property)
    {
        property = default;
        return this._logger.BindProperty(propertyName, value, destructureObjects, out property);
    }
}
beakona commented 3 months ago

I've made 1.0.36-pre prerelease that should work as expected.

tecAmoRaller commented 3 months ago

not work for me: error CS8623: Explicit application of 'System.Runtime.CompilerServices.NullableAttribute' is not allowed.

 /// <summary>
 /// Represents a type used to perform logging.
 /// </summary>
 /// <remarks>Aggregates most logging patterns to a single method.</remarks>
 public interface ILogger
 {
     /// <summary>
     /// Writes a log entry.
     /// </summary>
     void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter);

}

generated

        void Microsoft.Extensions.Logging.ILogger.Log<TState>(Microsoft.Extensions.Logging.LogLevel logLevel, Microsoft.Extensions.Logging.EventId eventId, [System.Runtime.CompilerServices.NullableAttribute(1)] TState state, System.Exception? exception, [System.Runtime.CompilerServices.NullableAttribute({1, 1, 2, 1})] System.Func<TState, System.Exception?, string> formatter) => this._melImpl.Log<TState>(logLevel, eventId, state, exception, formatter);

UPD and i have problem with [return: System.Runtime.CompilerServices.NullableAttribute] image

tecAmoRaller commented 3 months ago

and i want generate proxy for NLog.ILogger. Source code decorated JetBrains.Annotations.StructuredMessageTemplate attribute. I do not use JetBrains.Annotations image

i don`t understand, how we can handling unknown attributes.

tecAmoRaller commented 3 months ago

Mabye we can generate only System.Diagnostics.CodeAnalysis namespace attributes? @beakona

beakona commented 3 months ago

I've made changes. Only specific attributes are bing forwarded now. Here is the list:

  1. System.ObsoleteAttribute
  2. most of attributes from System.Diagnostics.CodeAnalysis namespace
tecAmoRaller commented 3 months ago

Sorry, not work for me. incorrect return attribute for properties. image

Please check pull request

tecAmoRaller commented 3 months ago

@beakona https://learn.microsoft.com/ru-ru/dotnet/csharp/language-reference/language-specification/attributes Check return value. Maybe we want exclude return target for property image

beakona commented 3 months ago

Yes, you are right. It is easier to skip return target for properties and indexers and fall back to expanded version of that property.. If we have following:

interface ITest
{
    int Count
    {
           [return: SomeForwardedAttribute]
           get;
    }
}

then we will not generate short version:

class Test : ITest
{
     [return: SomeForwardedAttribute]
     int Count => 1
}

but rather skip to full version:

class Test : ITest
{
     int Count
     {
          [return: SomeForwardedAttribute]
          get => 1
     }
}
tecAmoRaller commented 3 months ago

Check net 6 - all it`s work. Not worked, if i use netstandard2.0 targeting, because types on System.Diagnostics.CodeAnalysis non-public(( and dotnet show cs8769 warning

image

if we want translate internal attributes, we have compilation error CS0122 if we don`t ranslate internal attributes, we have cs8769.

See https://github.com/dotnet/roslyn/issues/39718

Today i use https://www.nuget.org/packages/PolySharp/ for netstandard2.0 code.

I think, we can generate System.Diagnostics.CodeAnalysis attributes for .net 2.0 targeting, or use external nuget packet at default references.

beakona commented 3 months ago

As I understood, you have following problems:

Nullability can be avoided by switching to newer language version. Add LangVersion in .csproj and reload that project

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    <LangVersion>8.0</LangVersion>
  </PropertyGroup>

Luckily, code analysis will work based on class names as long as you have new environment (IDE, compiler..). You can reference existing library that have such classes (but then you depends on foreign code) and also you can write it yourself. Here is roslyn implementation, you can copy paste desired classes in your codebase.

I've created new dummy solution with two projects. One is console app targeting .net 8.0 and other is class library targeting netstandard 2.0.

Inside class library I've added following

    public interface ITest
    {
        public bool TryParse(string s, [NotNullWhen(true)] out int? value);
    }

    public partial class Test
    {
        [BeaKona.AutoInterface(IncludeBaseInterfaces = true)]
        public ITest TestElement { get; set; } //warning: not populated...
    }

...

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Parameter, Inherited = false)]
    internal sealed class NotNullWhenAttribute : Attribute
    {
        /// <summary>Initializes the attribute with the specified return value condition.</summary>
        /// <param name="returnValue">
        /// The return value condition. If the method returns this value, the associated parameter will not be null.
        /// </param>
        public NotNullWhenAttribute(bool returnValue) => ReturnValue = returnValue;

        /// <summary>Gets the return value condition.</summary>
        public bool ReturnValue { get; }
    }
}

Inside console app I've added following

    static void Main(string[] args)
    {
        ITest test = new Test();
        if (test.TryParse("x", out var value))
        {
            int val = value.Value;//case1
        }
        else
        {
            int val = value.Value;//case2
        }
    }

I've also edited .csproj for class library and add lang version that supports nullable syntax

It compiles and branch labeled with case2 will be marked as invalid because value can be null there.

tecAmoRaller commented 3 months ago

if you declare NotNullWhenAttribute in your library, and use AutoInterface in the same library, you don`t have problem. if Test implementation declared in console application, you have CS8769 warning.

please, in your example, move Test from library to console app

image See please.

AutoInterface not generated internal attribute NotNullWhenAttribute from library, console app catch cs8769 warning, because generated method don`t have NotNullWhenAttribute

beakona commented 3 months ago

I've made following changes:

  1. have moved Test to console application
  2. have changed access modifier of NotNullWhenAttribute from internal to public
  3. added alias for class library inside console application project [by locating referenced library in Solution explorer and inside Properties I've added alias 'kok']
  4. in console application every mention of something from class library now have to go through alias, I've added following two lines:
    extern alias kok;
    using kok::MyTestClassLibrary;

after that, emited code looks like this:

bool kok::MyTestClassLibrary.ITest.Parse(string s, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out int? value)

It is erroneous, because at the moment, we are emitting attributes contextless (ie. without passing through type conversion mechanism)... thats a bug on my side.

To compiler it looks like we abandoned kok::System.Diagnostics.CodeAnalysis.NotNullWhenAttribute and added new attribute global::System.Diagnostics.CodeAnalysis.NotNullWhenAttribute and that is valid according to rules of implementing and overloading members, and code analysis works.

After fixing that bug and after emitting right type everything will be right, code analysis would continue to work as expected but only method 'signature' would be slightly different: pointing to kok:: target, rather than global::

I like this erroneous output more than pure one (that should be valid).

tecAmoRaller commented 3 months ago

I can`t changed access modifier of NotNullWhenAttribute, because library is nuget package Serliog (((

Today you filtering only public attributes. image

i think we can use public attributes AND "System.Diagnostics.CodeAnalysis" attribute. For example image

it`s work, if console app is net8. if we change net 8 console app to nestandard 2.0 library, we have a problem - CS0122

then after generation we can check existing attributes in project and generate attributes, if not exist for example

    if (<NotNullWhenAttribute required>)
    {
        var check = SdcaDetector.IsImplemented(compilation, "System.Diagnostics.CodeAnalysis.NotNullWhenAttribute");
        if (!check)
        {
            //TODO generate NotNullWhenAttribute attribute

        }
    }

static class SdcaDetector
{
   public static bool IsImplemented(Compilation compilation, string className)
{
    // This chunk of code is pretty inefficient and could be greatly improved!!
    INamedTypeSymbol[] allTypes = GetAllNamespaces(compilation)
        .SelectMany(t => t.GetTypeMembers())
        .SelectMany(AllNestedTypesAndSelf)
        .Where(p => p.DeclaredAccessibility == Accessibility.Public)
        .ToArray();

    return allTypes.Any(p => p.ToDisplayString() == className);

}

    private static IEnumerable<INamedTypeSymbol> AllNestedTypesAndSelf(this INamedTypeSymbol type)
    {
        yield return type;
        foreach (var typeMember in type.GetTypeMembers())
        {
            foreach (var nestedType in typeMember.AllNestedTypesAndSelf())
            {
                yield return nestedType;
            }
        }
    }

    private static ImmutableArray<INamespaceSymbol> GetAllNamespaces(Compilation compilation)
    {
        HashSet<INamespaceSymbol> seen = new HashSet<INamespaceSymbol>(SymbolEqualityComparer.Default);
        Queue<INamespaceSymbol> visit = new Queue<INamespaceSymbol>();
        visit.Enqueue(compilation.GlobalNamespace);

        do
        {
            INamespaceSymbol search = visit.Dequeue();
            seen.Add(search);

            foreach (INamespaceSymbol? space in search.GetNamespaceMembers())
            {
                if (space == null || seen.Contains(space))
                {
                    continue;
                }

                visit.Enqueue(space);
            }
        } while (visit.Count > 0);

        return [..seen];
    }

UPD Or we can show Diagnostic error, without generating System.Diagnostics.CodeAnalysis attributes. User can use Nuget packet with polyfils or implement attributes in code or use another source generator

tecAmoRaller commented 3 months ago

thanks, https://github.com/beakona/AutoInterface/releases/tag/v1.0.39-pre work for me.

tecAmoRaller commented 3 months ago

@beakona please make release version.

beakona commented 3 months ago

I've made some final changes prior to release..

tecAmoRaller commented 2 months ago

I want reopen issue.

Please, we want translate attributes for parameters

partial record TecProgDbConnection([property: BeaKona.AutoInterface(typeof(IDbConnection), IncludeBaseInterfaces = true)] NpgsqlConnection inner): ITecProgDbConnection
{
    public ValueTask DisposeAsync()
    {
        return inner.DisposeAsync();
    }
}
partial record TecProgDbConnection
{
    ...
    string System.Data.IDbConnection.ConnectionString
    {
        get => (this.inner as System.Data.IDbConnection)!.ConnectionString;
        set => (this.inner as System.Data.IDbConnection)!.ConnectionString = value;
    }
...
}

image image

beakona commented 2 months ago

I've made changes... Now, attributes attached to value parameter of each set method (for properties/indexers) [and add/remove methods for events] are being propagated to output..

I've discovered that value parameter for setter is the last parameter of that method... if we have following indexer:

string this[int a, [AllowNull] string b]
{
    get;
    [AllowNull]
    set;
}

then setter's signature would look like

void (*this)(int a, [AllowNull] string b, [AllowNull] string value)
tecAmoRaller commented 2 months ago

Please, make new version for nuget package.

tecAmoRaller commented 1 month ago

We have a problem with AllowNullAttribute

image I think, we can transform AllowNull and DisallowNull from setter to property.

image

For example, it`s work in npgsql image

See plz counterexample https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/attributes/nullable-analysis#preconditions-allownull-and-disallownull

beakona commented 1 month ago

I've made changes and I don't have time to test it... Prerelease v1.0.43-pre is on nuget..

tecAmoRaller commented 1 month ago

@beakona thanks. it`s work for me