dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.81k stars 3.2k forks source link

Add inheritdoc to migration files #10695

Closed MaklaCof closed 2 years ago

MaklaCof commented 6 years ago

It would be nice if dotnet ef migrations add would automatically add

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member

to the beginning of generated file and

#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

at the end of file. I mean both (migrations and designer).

For example:

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member

using Microsoft.EntityFrameworkCore.Migrations;
using System;
using System.Collections.Generic;

namespace OpPIS.Web.Hosting.Migrations
{
    public partial class Fix5 : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.AddColumn<int>(
                name: "ParentAnchorX",
                table: "VarDesign_Parameter-ImageManipulations",
                nullable: true);
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropColumn(
                name: "ParentAnchorX",
                table: "VarDesign_Parameter-ImageManipulations");
        }
    }
}

#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
ajcvickers commented 6 years ago

@MaklaCof We will have a think about adding XML comments to the generated code, but we don't plan to do it at the present time. Beyond that, we don't want to generate this pragma all the time because most people will not need it. However, there is a similar discussion about auto-generated in issue #10203. I posted a code snippet in that issue showing how to override the generator to add a line--this should also work for the pragma.

bobvandevijver commented 6 years ago

For reference in this ticket, this is what I used to generate the pragma's:

public class FooMigrationsGenerator : CSharpMigrationsGenerator
{
    private const string PragmaWarningDisable = @"#pragma warning disable 1591";
    private const string PragmaWarningRestore = @"#pragma warning restore 1591";

    public FooMigrationsGenerator(
        MigrationsCodeGeneratorDependencies dependencies, CSharpMigrationsGeneratorDependencies csharpDependencies)
        : base(dependencies, csharpDependencies)
    {
    }

    public override string GenerateMigration(string migrationNamespace, string migrationName,
                                             IReadOnlyList<MigrationOperation> upOperations,
                                             IReadOnlyList<MigrationOperation> downOperations)
    {
        return PragmaWarningDisable
               + Environment.NewLine
               + base.GenerateMigration(migrationNamespace, migrationName, upOperations, downOperations)
               + Environment.NewLine
               + PragmaWarningRestore
               + Environment.NewLine;
    }

    public override string GenerateMetadata(string migrationNamespace, Type contextType, string migrationName,
                                            string migrationId, IModel targetModel)
        => PragmaWarningDisable
           + Environment.NewLine
           + base.GenerateMetadata(migrationNamespace, contextType, migrationName, migrationId, targetModel)
           + Environment.NewLine
           + PragmaWarningRestore
           + Environment.NewLine;
}

Together with this class somewhere in the starting assembly:

public class DesignTimeServices : IDesignTimeServices
{
    /// <inheritdoc />
    public void ConfigureDesignTimeServices(IServiceCollection services)
    {
        // Register custom migration generator which adds a pragma disable for xml-comments
        services.AddSingleton<IMigrationsCodeGenerator, FooMigrationsGenerator>();
    }
}
MaklaCof commented 4 years ago

This doesn't work anymore with version 3.1.1. Namespace Microsoft.EntityFrameworkCore.Migrations.Design can not be found.

ErikEJ commented 4 years ago

@MaklaCof This? https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dip

MaklaCof commented 4 years ago

@ErikEJ Does anything else changed?

Class CSharpEntityTypeGenerator is found in using Microsoft.EntityFrameworkCore.Scaffolding.Internal;

But there are no methods: GenerateMigration, GenerateSnapshot, GenerateMetadata.
Only: image

My Packages are:

    <ItemGroup>
        <PackageReference Include="Autofac" Version="4.8.1" />
        <PackageReference Include="Autofac.Extensions.DependencyInjection" Version="4.3.0" />
        <PackageReference Include="MediatR" Version="5.1.0" />
        <PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="3.1.1" />
        <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="3.1.1" />
        <PackageReference Include="Microsoft.AspNetCore.SignalR" Version="1.1.0" />
        <PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson" Version="3.1.1" />
        <PackageReference Include="Microsoft.AspNetCore.SpaServices.Extensions" Version="3.1.1" />
        <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8">
          <PrivateAssets>all</PrivateAssets>
          <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
        </PackageReference>
        <PackageReference Include="Microsoft.EntityFrameworkCore" Version="3.1.1" />
        <PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="3.1.1">
          <PrivateAssets>all</PrivateAssets>
            <!--
            Without this, Migrations.cs file doesn't compile.
            https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dip
            <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
          -->
        </PackageReference>
        <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="3.1.1">
          <PrivateAssets>all</PrivateAssets>
          <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
        </PackageReference>
        <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="3.1.0" />
        <PackageReference Include="OpenIddict" Version="2.0.1" />
        <PackageReference Include="OpenIddict.EntityFrameworkCore" Version="2.0.1" />
        <PackageReference Include="OpenIddict.Mvc" Version="2.0.1" />
        <PackageReference Include="Swashbuckle.AspNetCore" Version="4.0.1" />
        <PackageReference Include="Z.EntityFramework.Extensions.EFCore" Version="3.0.26" />
        <PackageReference Include="Z.EntityFramework.Plus.EFCore" Version="3.0.29" />
    </ItemGroup>
ErikEJ commented 4 years ago

Possibly, since you are using Internal classes

Ablinne commented 4 years ago

I think this should really be implemented. The counter-arguments

are imho invalid, because:

roji commented 4 years ago

Opening to re-discuss in triage.

ajcvickers commented 4 years ago

This does seem reasonable to do. We should consider using inheritdoc.

ajcvickers commented 3 years ago

Note from triage: use inheritdoc for this.

jeancallisti commented 3 years ago

Please do consider this for next release. XML comments are not the only ones causing annoying warnings.

Each new Migration file triggers several warnings. I need to disable at least those manually:

pragma warning disable CA1814 // Prefer jagged arrays over multidimensional

pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type.

CA1814 is mostly caused by seeding using "HasData" CS8625 is caused by the use of anonymous types when creating nullable Owned entities.

roji commented 3 years ago

@jeancallisti CS8625 (nullability) is covered by #18950. But I'll look into CA1814 when doing this.