dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.11k stars 4.04k forks source link

IDE0060 flags partial methods used by source generators #75700

Open TubaMirabilis opened 4 weeks ago

TubaMirabilis commented 4 weeks ago

I have been using the Riok.Mapperly library and during development everything is fine.

However when running 'dotnet format --verify-no-changes' on the repository in GitHub Actions (or on my PC) the errors appear and the code quality check fails.

I am using 'dotnet format --verify-no-changes --exclude-diagnostics IDE0060' for now but it's not very elegant.

Commands like 'dotnet build' seem to be okay.

Some user-written code:

[Mapper]
public static partial class DeviceMapper
{
    [MapProperty("Name.Value", "Name")]
    public static partial DeviceDto MapDeviceToDeviceDto(Device d);
}
gavar commented 3 weeks ago

I've got the same problem recently this week.

CyrusNajmabadi commented 2 weeks ago

Where is IDE0060 reported on?

jakoss commented 1 week ago

Similar issue

[Mapper]
public static partial class FilePermissionMapper
{
    [MapperRequiredMapping(RequiredMappingStrategy.Target)]
    public static partial FilePermissionResponse ToProtoResponse(this FilePermission filePermission);

    [MapperRequiredMapping(RequiredMappingStrategy.Source)]
    public static partial FilePermission FromProtoRequest(AddFilePermissionRequest addFilePermissionRequest);
}

dotnet format --verify-no-changes reports IDE0060 in filePermission parameter and addFilePermissionRequest parameter.

Message "error IDE0060: Remove unused parameter 'addFilePermissionRequest' if it is not part of a shipped public API"

CyrusNajmabadi commented 1 week ago

If the generator is not using that parameter, it makes sense to still report the issue afaict. Why is the generator not using it? And if it truly doesn't use it, it can still emit something like _ = addFilePermissionRequest to simulate using it.

But something seems very fishy here. Why have the parameter at all if not to be used?

jakoss commented 1 week ago

It is being used:

public static partial class FilePermissionMapper
    {
        [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.1.0.0")]
        public static partial global::Files.FilePermissionResponse ToProtoResponse(this global::Files.Data.Entities.FilePermission filePermission)
        {
            var target = new global::Files.FilePermissionResponse();
            target.Id = (global::Common.Id)filePermission.Id;
            target.FileId = (global::Common.Id)filePermission.FileId;
            target.ObjectId = filePermission.ObjectId;
            target.PermissionLevel = filePermission.PermissionLevel;
            return target;
        }

        [global::System.CodeDom.Compiler.GeneratedCode("Riok.Mapperly", "4.1.0.0")]
        public static partial global::Files.Data.Entities.FilePermission FromProtoRequest(global::Files.AddFilePermissionRequest addFilePermissionRequest)
        {
            var target = new global::Files.Data.Entities.FilePermission()
            {
                ObjectId = addFilePermissionRequest.ObjectId ?? throw new System.ArgumentNullException(nameof(addFilePermissionRequest.ObjectId)),
            };
            target.PermissionLevel = addFilePermissionRequest.PermissionLevel;
            return target;
        }
    }
jakoss commented 1 week ago

Another workaround for now is to add this to the .editorconfig

[*Mapper.cs]
dotnet_diagnostic.ide0060.severity = none
CyrusNajmabadi commented 1 week ago

It is being used:

Does this repro absent SGs being used? Can you just make a partial method with two parts, and have it be used, and still see thsi arise? I'm getting thrown off by the SG part of the title :)

jakoss commented 1 week ago

This works without issues:

public static partial class FilePermissionMapper
{
    public static partial FilePermissionResponse ToProtoResponse(this FilePermission filePermission);

    public static partial FilePermission FromProtoRequest(AddFilePermissionRequest addFilePermissionRequest);
}

public static partial class FilePermissionMapper
{
    public static partial FilePermissionResponse ToProtoResponse(this FilePermission filePermission)
    {
        return new FilePermissionResponse();
    }

    public static partial FilePermission FromProtoRequest(AddFilePermissionRequest addFilePermissionRequest)
    {
        return new FilePermission
        {
            ObjectId = "test"
        };
    }
}

So it seems to be connected to the source generation here

CyrusNajmabadi commented 1 week ago

Ok. let me take a looksie.