Efcore: Consider addition of <auto-generated /> to Migration file.

[vitalybibikov]

Currently EF Core 2.0 generates migration files without tag
and only creates it for *.designer files.

Such behavior breaks compilation of a project when scripts are being used to create and populate the migrations. (with stylecop added to the proj).
So we have to add manually such a tag each time we are starting out supposed to be automation scripts.

Thanks

[ajcvickers]

@EvilAvenger The scaffolding migrations are intended to be edited. The add-migration command will do it's best to scaffold the correct migration, but there are things that it doesn't support and things it does not know about that may need to be added. We will discuss this again to be sure, but in our last discussion on this we decided that the semantics of auto-generated didn't match this.

[bricelam]

Which StyleCop errors are you seeing?

[vitalybibikov]

Currently I see only SA1208 but it may vary in some cases.

[Nihlus]

While the files are intended to be edited they are quite clearly auto-generated code that, in their original form, do not neccesarily conform to a project's style settings. Furthermore, the lack of the auto-generated tag prevents migrations from being successfully created if StyleCop is set to treat its inspections as errors, as the build fails and the generated code is not added. This is not only an issue from a scripting standpoint, but it also prevents manual postgeneration addition of the tag.

I have implemented the addition of the tag, along with modifying its corresponding unit test, in this branch: https://github.com/Nihlus/EntityFrameworkCore/tree/migration-auto-generated-tag

As for which errors are triggered, it varies depending on the complexity of the migration, but for an initial creation of a simple-ish SQLite database, I am seeing SA1027 (differing tab and space usage), SA1633 (migrations do not have file headers). Furthermore, when applying the migration via ef database update, I am also seeing SA1208 (System usings before others) and SA1600 (elements should be documented).

Additionally, I am seeing SA1200 (usings should be inside namespace declarations), but my project is configured for the opposite. Outside of generating the migrations, that works without issue, so there's probably another problem with how the migration tool compiles the code and its working directory.

In any case, the fact that the migrations are intended to be edited after the fact does not change their generated nature, and they should be labelled as such. It certainly wouldn't do any harm.

[bricelam]

Could you share the code that violates SA1027 (tabs vs spaces)? We shouldn't ever be generating tabs... (unless they came from user input)

I think everyone is on board for fixing SA1208 (System namespaces first); all of the dotnet new templates do this.

On the other hand, the templates violate SA1200, SA1600 & SA1633, so we'd probably be doing more harm than good by fixing those.

[ajcvickers]

@Nihlus We discussed this again and again came to the conclusion that auto-generated is not appropriate for a class that is scaffolded as a starting point with the expectation that it will then be edited. However, it is possible to create a custom code-generator that will add auto-generated if you really need it. For example:
```C#
public class CustomDesignTimeServices : IDesignTimeServices
{
public void ConfigureDesignTimeServices(IServiceCollection serviceCollection)
=> serviceCollection.AddSingleton();
}

public class CustomCSharpMigrationsGenerator : CSharpMigrationsGenerator
{
public CustomCSharpMigrationsGenerator(
MigrationsCodeGeneratorDependencies dependencies,
CSharpMigrationsGeneratorDependencies csharpDependencies)
: base(dependencies, csharpDependencies)
{
}

public override string GenerateMigration(
    string migrationNamespace,
    string migrationName,
    IReadOnlyList<MigrationOperation> upOperations,
    IReadOnlyList<MigrationOperation> downOperations)
    => @"// <auto-generated />"
       + Environment.NewLine
       + base.GenerateMigration(migrationNamespace, migrationName, upOperations, downOperations);

}
`` TheIDesignTimeServices` implementation needs to be in the startup assembly and will then be picked up automatically.

[ajcvickers]

See also new issues #10226 (make it easy to add custom header) and #10225 (put System namespaces first).

[Nihlus]

@ajcvickers While I still disagree (the code is generated, why not mark it as such until it no longer is, wouldn't impede or hurt anything, etc), thank you very much for the example. I'll be sure to implement that into my project - will it be automatically discovered? I look forward to seeing what you come up with for #10226.

@bricelam SA1027 is being raised because my project is configured to use tabs (I know, I know...). SA1200 should not be raised, since it contradicts the project settings and the generated files are conformant, at least as far as my configuration is concerned.

Either way, it falls back to my previous arguments pertaining to the scaffolding's generated nature. The auto-generated tag is explicitly intended for this use case - marking generated code so that it can be treated as such until such a time that it has been manually reviewed or edited, if that is intended or required.

[bricelam]

Our fear is that <auto-generated /> also implies the file will be re-generated by the tool at some point and any edits will be lost.

[Nihlus]

@bricelam I see your point, however, that is easily remedied by being clear from the outset that the generated migrations are intended for post-generation edits, and one could even go so far as adding a comment after the tag that explicitly tells the user that the file is intended to be edited and will not be regenerated.

Beyond that, from the way migrations work, it's fairly apparent that they are a one-time deal. Even if it wasn't, it's nothing documentation and a short comment won't explain.

[sharwell]

I would agree with the conclusion here (https://github.com/aspnet/EntityFrameworkCore/issues/10203#issuecomment-341462735). However, since these files are expected to conform to project code style, I would expect the code generator to at minimum account for the current .editorconfig configuration, which includes things like brace placement, tabs/spaces, and the use of this. for instance references.

The upcoming Code Cleanup feature may end up providing the underlying functionality for this formatting step without needing to change the generator itself.

[artyomabrahamyan]

@ajcvickers @bricelam
What if I am using IDesignTimeDbContextFactory interface to create my migrations in the separate class library. Where should I inject my CustomCSharpMigrationsGenerator?