Skip to content

Commit

Permalink
Add <inheritdoc /> in migrations (#27268)
Browse files Browse the repository at this point in the history
And suppress CA1814 if multidimensional seed data is present.

Closes #10695
  • Loading branch information
roji authored Jan 28, 2022
1 parent 29e3969 commit a204b62
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 12 deletions.
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@
<MicrosoftExtensionsLoggingVersion>7.0.0-alpha.1.22073.5</MicrosoftExtensionsLoggingVersion>
</PropertyGroup>
<PropertyGroup Label="Other dependencies">
<MicrosoftCodeAnalysisVersion>3.7.0</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion>4.0.1</MicrosoftCodeAnalysisVersion>
</PropertyGroup>
</Project>
4 changes: 2 additions & 2 deletions src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ private static void AnalyzeNode(OperationAnalysisContext context)
case OperationKind.MethodReference:
AnalyzeMember(context, ((IMethodReferenceOperation)context.Operation).Method);
break;
case OperationKind.ObjectCreation:
AnalyzeMember(context, ((IObjectCreationOperation)context.Operation).Constructor);
case OperationKind.ObjectCreation when ((IObjectCreationOperation)context.Operation).Constructor is { } constructor:
AnalyzeMember(context, constructor);
break;
case OperationKind.Invocation:
AnalyzeInvocation(context, (IInvocationOperation)context.Operation);
Expand Down
27 changes: 27 additions & 0 deletions src/EFCore.Design/Migrations/Design/CSharpMigrationsGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ public override string GenerateMigration(
.AppendLine()
.AppendLine("#nullable disable");

// Suppress "Prefer jagged arrays over multidimensional" when we have a seeding operation with a multidimensional array
if (HasMultidimensionalArray(upOperations.Concat(downOperations)))
{
builder
.AppendLine()
.AppendLine("#pragma warning disable CA1814 // Prefer jagged arrays over multidimensional");
}

if (!string.IsNullOrEmpty(migrationNamespace))
{
builder
Expand All @@ -88,11 +96,13 @@ public override string GenerateMigration(
}

builder
.AppendLine("/// <inheritdoc />")
.Append("public partial class ").Append(Code.Identifier(migrationName)).AppendLine(" : Migration")
.AppendLine("{");
using (builder.Indent())
{
builder
.AppendLine("/// <inheritdoc />")
.AppendLine("protected override void Up(MigrationBuilder migrationBuilder)")
.AppendLine("{");
using (builder.Indent())
Expand All @@ -104,6 +114,7 @@ public override string GenerateMigration(
.AppendLine()
.AppendLine("}")
.AppendLine()
.AppendLine("/// <inheritdoc />")
.AppendLine("protected override void Down(MigrationBuilder migrationBuilder)")
.AppendLine("{");
using (builder.Indent())
Expand Down Expand Up @@ -191,6 +202,7 @@ public override string GenerateMetadata(
using (builder.Indent())
{
builder
.AppendLine("/// <inheritdoc />")
.AppendLine("protected override void BuildTargetModel(ModelBuilder modelBuilder)")
.AppendLine("{")
.DecrementIndent()
Expand Down Expand Up @@ -313,4 +325,19 @@ public override string GenerateSnapshot(

return builder.ToString();
}

private bool HasMultidimensionalArray(IEnumerable<MigrationOperation> operations)
{
return operations.Any(
o =>
(o is InsertDataOperation insertDataOperation
&& IsMultidimensional(insertDataOperation.Values))
|| (o is UpdateDataOperation updateDataOperation
&& (IsMultidimensional(updateDataOperation.Values) || IsMultidimensional(updateDataOperation.KeyValues)))
|| (o is DeleteDataOperation deleteDataOperation
&& IsMultidimensional(deleteDataOperation.KeyValues)));

static bool IsMultidimensional(Array array)
=> array.GetLength(0) > 1 && array.GetLength(1) > 1;
}
}
1 change: 1 addition & 0 deletions src/Shared/ReferenceEqualityComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Runtime.CompilerServices;

#nullable enable
#pragma warning disable RS1024 // Compare symbols correctly (for Roslyn analyzers only)

namespace Microsoft.EntityFrameworkCore.Internal;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,10 @@ public void Migrations_compile()
namespace MyNamespace
{
/// <inheritdoc />
public partial class MyMigration : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.Sql(""-- TEST"")
Expand All @@ -523,6 +525,7 @@ protected override void Up(MigrationBuilder migrationBuilder)
values: new object[] { 1, null, -1 });
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
Expand Down Expand Up @@ -571,6 +574,7 @@ namespace MyNamespace
[Migration(""20150511161616_MyMigration"")]
partial class MyMigration
{
/// <inheritdoc />
protected override void BuildTargetModel(ModelBuilder modelBuilder)
{
#pragma warning disable 612, 618
Expand Down Expand Up @@ -608,7 +612,8 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder)
BuildReference.ByName("Microsoft.EntityFrameworkCore"),
BuildReference.ByName("Microsoft.EntityFrameworkCore.Relational")
},
Sources = { { "Migration.cs", migrationCode }, { "MigrationSnapshot.cs", migrationMetadataCode } }
Sources = { { "Migration.cs", migrationCode }, { "MigrationSnapshot.cs", migrationMetadataCode } },
EmitDocumentationDiagnostics = true
};

var assembly = build.BuildInMemory();
Expand Down Expand Up @@ -1006,6 +1011,50 @@ public void Namespaces_imported_for_delete_data()
Assert.Contains("using System.Text.RegularExpressions;", migration);
}

[ConditionalFact]
public void Multidimensional_array_warning_is_suppressed_for_multidimensional_seed_data()
{
var generator = CreateMigrationsCodeGenerator();

var migration = generator.GenerateMigration(
"MyNamespace",
"MyMigration",
new[]
{
new DeleteDataOperation
{
Table = "MyTable",
KeyColumns = new[] { "Id" },
KeyValues = new object[,] { { 1, 2 }, { 3, 4 } }
}
},
Array.Empty<MigrationOperation>());

Assert.Contains("#pragma warning disable CA1814", migration);
}

[ConditionalFact]
public void Multidimensional_array_warning_is_not_suppressed_for_unidimensional_seed_data()
{
var generator = CreateMigrationsCodeGenerator();

var migration = generator.GenerateMigration(
"MyNamespace",
"MyMigration",
new[]
{
new DeleteDataOperation
{
Table = "MyTable",
KeyColumns = new[] { "Id" },
KeyValues = new object[,] { { 1, 2 } }
}
},
Array.Empty<MigrationOperation>());

Assert.DoesNotContain("#pragma warning disable CA1814", migration);
}

private static IMigrationsCodeGenerator CreateMigrationsCodeGenerator()
{
var testAssembly = typeof(CSharpMigrationsGeneratorTest).Assembly;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class BuildSource
public string TargetDir { get; set; }
public Dictionary<string, string> Sources { get; set; } = new();
public bool NullableReferenceTypes { get; set; }
public bool EmitDocumentationDiagnostics { get; set; }

public BuildFileResult Build()
{
Expand All @@ -46,7 +47,13 @@ public BuildFileResult Build()

var compilation = CSharpCompilation.Create(
projectName,
Sources.Select(s => SyntaxFactory.ParseSyntaxTree(s.Value).WithFilePath(s.Key)),
Sources.Select(
s => SyntaxFactory.ParseSyntaxTree(
text: s.Value,
path: s.Key,
options: new CSharpParseOptions(
LanguageVersion.Latest,
EmitDocumentationDiagnostics ? DocumentationMode.Diagnose : DocumentationMode.Parse))),
references,
CreateOptions());

Expand Down Expand Up @@ -78,18 +85,40 @@ public Assembly BuildInMemory()

var compilation = CSharpCompilation.Create(
projectName,
Sources.Select(s => SyntaxFactory.ParseSyntaxTree(s.Value).WithFilePath(s.Key)),
Sources.Select(
s => SyntaxFactory.ParseSyntaxTree(
text: s.Value,
path: s.Key,
options: new CSharpParseOptions(
LanguageVersion.Latest,
EmitDocumentationDiagnostics ? DocumentationMode.Diagnose : DocumentationMode.Parse))),
references,
CreateOptions());

var diagnostics = compilation.GetDiagnostics();
if (!diagnostics.IsEmpty)
{
throw new InvalidOperationException(
$@"Build failed.
First diagnostic:
{diagnostics[0]}
Location:
{diagnostics[0].Location.SourceTree?.GetRoot().FindNode(diagnostics[0].Location.SourceSpan)}
All diagnostics:
{string.Join(Environment.NewLine, diagnostics)}");
}

Assembly assembly;
using (var stream = new MemoryStream())
{
var result = compilation.Emit(stream);
if (!result.Success)
{
throw new InvalidOperationException(
$@"Build failed:
$@"Failed to emit compilation:
{string.Join(Environment.NewLine, result.Diagnostics)}");
}

Expand All @@ -107,12 +136,19 @@ private CSharpCompilationOptions CreateOptions()
generalDiagnosticOption: ReportDiagnostic.Error,
specificDiagnosticOptions: new Dictionary<string, ReportDiagnostic>
{
// Displays the text of a warning defined with the #warning directive
{ "CS1030", ReportDiagnostic.Suppress },

// Assuming assembly reference "Assembly Name #1" matches "Assembly Name #2", you may need to supply runtime policy
{ "CS1701", ReportDiagnostic.Suppress },
{ "CS1702", ReportDiagnostic.Suppress }, // Always thrown for .NET Core
{
"CS1705", ReportDiagnostic.Suppress
}, // Assembly 'AssemblyName1' uses 'TypeName' which has a higher version than referenced assembly 'AssemblyName2'
{ "CS8019", ReportDiagnostic.Suppress } // Unnecessary using directive.

// Assuming assembly reference "Assembly Name #1" matches "Assembly Name #2", you may need to supply runtime policy
{ "CS1702", ReportDiagnostic.Suppress },

// Assembly 'AssemblyName1' uses 'TypeName' which has a higher version than referenced assembly 'AssemblyName2'
{ "CS1705", ReportDiagnostic.Suppress },

// Unnecessary using directive.
{ "CS8019", ReportDiagnostic.Suppress }
});
}

0 comments on commit a204b62

Please sign in to comment.