json-api-dotnet / JsonApiDotNetCore

A framework for building JSON:API compliant REST APIs using ASP.NET and Entity Framework Core.
https://www.jsonapi.net
MIT License
679 stars 159 forks source link

Different dotnet tool run jb cleanupcode results locally and on appveyor #1104

Closed ThomasBarnekow closed 2 years ago

ThomasBarnekow commented 2 years ago

DESCRIPTION

The results of the following build tool are much different on my local machine and the appveyor build server:

dotnet tool run jb cleanupcode "JsonApiDotNetCore.sln"  --include=";src/JsonApiDotNetCore/Configuration/NoSqlServiceCollectionExtensions.cs;src/JsonApiDotNetCore/Queries/INoSqlQueryLayerComposer.cs;src/JsonApiDotNetCore/Queries/NoSqlQueryLayerComposer.cs;src/JsonApiDotNetCore/Resources/Annotations/NoSqlHasForeignKeyAttribute.cs;src/JsonApiDotNetCore/Resources/Annotations/NoSqlOwnsManyAttribute.cs;src/JsonApiDotNetCore/Resources/Annotations/NoSqlResourceAttribute.cs;src/JsonApiDotNetCore/Services/NoSqlResourceService.cs" --profile --profile="JADNC Full Cleanup" --properties:Configuration=Release --verbosity=WARN -dsl=GlobalAll -dsl=SolutionPersonal -dsl=ProjectPersonal

On the build server, the expected formatting is this:

public NoSqlResourceService(IResourceRepositoryAccessor repositoryAccessor, IQueryLayerComposer sqlQueryLayerComposer,
    IPaginationContext paginationContext, IJsonApiOptions options, ILoggerFactory loggerFactory, IJsonApiRequest request,
    IResourceChangeTracker<TResource> resourceChangeTracker, IResourceDefinitionAccessor resourceDefinitionAccessor,
    INoSqlQueryLayerComposer queryLayerComposer, IResourceGraph resourceGraph, IEvaluatedIncludeCache evaluatedIncludeCache)
{
    // Further details omitted.
}

When run locally on my machine, the above code is reformatted as follows (which is in line with what it looked like before and what was rejected by the build server):

public NoSqlResourceService(
    IResourceRepositoryAccessor repositoryAccessor,
    IQueryLayerComposer sqlQueryLayerComposer,
    IPaginationContext paginationContext,
    IJsonApiOptions options,
    ILoggerFactory loggerFactory,
    IJsonApiRequest request,
    IResourceChangeTracker<TResource> resourceChangeTracker,
    IResourceDefinitionAccessor resourceDefinitionAccessor,
    INoSqlQueryLayerComposer queryLayerComposer,
    IResourceGraph resourceGraph,
    IEvaluatedIncludeCache evaluatedIncludeCache)
{
    // Further details omitted.
}

This has lead to build failures in #1103.

STEPS TO REPRODUCE

Run the above command line on those or other files.

EXPECTED BEHAVIOR

Both locally and on appveyor, the expected code format is identical. When using ReSharper in Visual Studio, the code cleanup creates the expected code format.

If I had a choice, the second format (which is produced by ReSharper using the "JADNC Full Cleanup" on my local machine but unfortunately rejected on appveyor) is the expected format.

ACTUAL BEHAVIOR

When run locally and on appveyor, the above command produces different results, leading to unexpected build failures.

VERSIONS USED

bart-degreed commented 2 years ago

Hi @ThomasBarnekow, sad to hear you're struggling with this. I'm using Visual Studio 2019 with Resharper on Windows 10 and @maurei uses Rider on Mac. We use the command-line runners regularly and have not experienced this problem. I can't think of a reason that explains this. What AppVeyor reports is the style we use.

Some tips that might help:

ThomasBarnekow commented 2 years ago

Hi @bart-degreed, I am also using Visual Studio 2019 with ReSharper on Windows 10. I used the PowerShell script, which installed the jb tool. After that, it was also able to directly run the dotnet tool run jb command.

If the command disables any other settings layer and only uses the "solution team shared" part of the settings, I also don't understand how this happens. I can tell you that both Visual Studio and the command line produce the exact same result, which is in line with how I tend to lay out long parameter lists. Thus, is it possible that certain settings are not ignored?

bart-degreed commented 2 years ago

I don't know, could be a bug. Try resetting to the defaults.

ThomasBarnekow commented 2 years ago

I think I've been successful with the following command, which I tried on just one file:

dotnet tool run jb cleanupcode "JsonApiDotNetCore.sln" --include=".\src\JsonApiDotNetCore\Services\NoSqlResourceService.cs" --profile="JADNC Full Cleanup" --no-buildin-settings  --settings=".\JsonApiDotNetCore.sln.DotSettings"

Note the --no-buildin-settings (sic) option. As per the documentation, this disables GlobalAll, GlobalPerProduct, SolutionShared, SolutionPersonal, ProjectShared, and ProjectPersonal. In your command, you are not disabling GlobalPerProduct, which is the culprit in this case.

It seems that your settings are silent on how long parameter lists are laid out while I have a non-disabled global setting that specifies how this is done. This is applied by ReSharper and leads to the difference.

Not disabling that setting looks like a "bug" in your original command. To fix this, you should simply disable GlobalPerProduct as well by adding -dsl=GlobalPerProduct to the command.

I have also successfully used the following command, which does not specify the settings explicitly but only excludes the relevant settings and implicitly leaves SolutionShared and ProjectShared settings:

dotnet tool run jb cleanupcode "JsonApiDotNetCore.sln" --include=".\src\JsonApiDotNetCore\Services\NoSqlResourceService.cs" --profile="JADNC Full Cleanup" -dsl=GlobalAll -dsl=GlobalPerProduct -dsl=SolutionPersonal -dsl=ProjectPersonal
bart-degreed commented 2 years ago

Thanks for sorting this out, I've created a PR to fix this.