From 2ea608146a156760f197b94e559183f7150ab0d8 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Mon, 24 Jan 2022 15:56:55 +0100 Subject: [PATCH] Add in migrations And suppress CA1814 if multidimensional seed data is present. Closes #10695 --- eng/Versions.props | 2 +- .../InternalUsageDiagnosticAnalyzer.cs | 4 +- .../Design/CSharpMigrationsGenerator.cs | 27 ++++++++++ src/Shared/ReferenceEqualityComparer.cs | 1 + .../Design/CSharpMigrationsGeneratorTest.cs | 51 +++++++++++++++++- .../TestUtilities/BuildSource.cs | 52 ++++++++++++++++--- 6 files changed, 125 insertions(+), 12 deletions(-) diff --git a/eng/Versions.props b/eng/Versions.props index ce67696d706..c93a20ec1b5 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -28,6 +28,6 @@ 7.0.0-alpha.1.22066.4 - 3.7.0 + 4.0.1 diff --git a/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs b/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs index 39aecf7ff7e..b4d65f6a132 100644 --- a/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs +++ b/src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs @@ -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); diff --git a/src/EFCore.Design/Migrations/Design/CSharpMigrationsGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpMigrationsGenerator.cs index 96a6652eeda..9b40b34328a 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpMigrationsGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpMigrationsGenerator.cs @@ -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 @@ -88,11 +96,13 @@ public override string GenerateMigration( } builder + .AppendLine("/// ") .Append("public partial class ").Append(Code.Identifier(migrationName)).AppendLine(" : Migration") .AppendLine("{"); using (builder.Indent()) { builder + .AppendLine("/// ") .AppendLine("protected override void Up(MigrationBuilder migrationBuilder)") .AppendLine("{"); using (builder.Indent()) @@ -104,6 +114,7 @@ public override string GenerateMigration( .AppendLine() .AppendLine("}") .AppendLine() + .AppendLine("/// ") .AppendLine("protected override void Down(MigrationBuilder migrationBuilder)") .AppendLine("{"); using (builder.Indent()) @@ -191,6 +202,7 @@ public override string GenerateMetadata( using (builder.Indent()) { builder + .AppendLine("/// ") .AppendLine("protected override void BuildTargetModel(ModelBuilder modelBuilder)") .AppendLine("{") .DecrementIndent() @@ -313,4 +325,19 @@ public override string GenerateSnapshot( return builder.ToString(); } + + private bool HasMultidimensionalArray(IEnumerable 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; + } } diff --git a/src/Shared/ReferenceEqualityComparer.cs b/src/Shared/ReferenceEqualityComparer.cs index 46fc92c45c1..0a1dec1a0c9 100644 --- a/src/Shared/ReferenceEqualityComparer.cs +++ b/src/Shared/ReferenceEqualityComparer.cs @@ -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; diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs index f83be456152..e77b22371af 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.cs @@ -499,8 +499,10 @@ public void Migrations_compile() namespace MyNamespace { + /// public partial class MyMigration : Migration { + /// protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.Sql(""-- TEST"") @@ -523,6 +525,7 @@ protected override void Up(MigrationBuilder migrationBuilder) values: new object[] { 1, null, -1 }); } + /// protected override void Down(MigrationBuilder migrationBuilder) { @@ -571,6 +574,7 @@ namespace MyNamespace [Migration(""20150511161616_MyMigration"")] partial class MyMigration { + /// protected override void BuildTargetModel(ModelBuilder modelBuilder) { #pragma warning disable 612, 618 @@ -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(); @@ -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()); + + 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()); + + Assert.DoesNotContain("#pragma warning disable CA1814", migration); + } + private static IMigrationsCodeGenerator CreateMigrationsCodeGenerator() { var testAssembly = typeof(CSharpMigrationsGeneratorTest).Assembly; diff --git a/test/EFCore.Relational.Specification.Tests/TestUtilities/BuildSource.cs b/test/EFCore.Relational.Specification.Tests/TestUtilities/BuildSource.cs index be678bdf14b..a3aa2d5068f 100644 --- a/test/EFCore.Relational.Specification.Tests/TestUtilities/BuildSource.cs +++ b/test/EFCore.Relational.Specification.Tests/TestUtilities/BuildSource.cs @@ -23,6 +23,7 @@ public class BuildSource public string TargetDir { get; set; } public Dictionary Sources { get; set; } = new(); public bool NullableReferenceTypes { get; set; } + public bool EmitDocumentationDiagnostics { get; set; } public BuildFileResult Build() { @@ -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()); @@ -78,10 +85,32 @@ 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()) { @@ -89,7 +118,7 @@ public Assembly BuildInMemory() if (!result.Success) { throw new InvalidOperationException( - $@"Build failed: + $@"Failed to emit compilation: {string.Join(Environment.NewLine, result.Diagnostics)}"); } @@ -107,12 +136,19 @@ private CSharpCompilationOptions CreateOptions() generalDiagnosticOption: ReportDiagnostic.Error, specificDiagnosticOptions: new Dictionary { + // 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 } }); }