-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add <inheritdoc /> in migrations #27268
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this related to inheritdoc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It isn't - it's another thing in the same area that's tracked by that same issue (see #10695 (comment)). Do you prefer I split it out, or think we shouldn't do it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should at least discuss. I really hate adding suppressions, especially for overzealous warnings like this. Is this a modern analyzer warning? If so, what is the justification for it? If it's an old FxCop warning then... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This rule is reported by the standard .NET analyzers that started shipping as part of the SDK in .NET 5.0 (see docs). It's indeed not enabled by default, but anyone enabling e.g. all the "performance" category diagnostics gets it. I also hate adding suppressions, which is why I went to the extra trouble of adding it only where we actually generate a multidimensional array. I do think it reduces a bit of needless friction for users who enable most of the rules, but if you're against it we can discuss and remove. |
||
{ | ||
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("/// <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()) | ||
|
@@ -104,6 +114,7 @@ public override string GenerateMigration( | |
.AppendLine() | ||
.AppendLine("}") | ||
.AppendLine() | ||
.AppendLine("/// <inheritdoc />") | ||
.AppendLine("protected override void Down(MigrationBuilder migrationBuilder)") | ||
.AppendLine("{"); | ||
using (builder.Indent()) | ||
|
@@ -191,6 +202,7 @@ public override string GenerateMetadata( | |
using (builder.Indent()) | ||
{ | ||
builder | ||
.AppendLine("/// <inheritdoc />") | ||
.AppendLine("protected override void BuildTargetModel(ModelBuilder modelBuilder)") | ||
.AppendLine("{") | ||
.DecrementIndent() | ||
|
@@ -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; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move the following comment to here?
efcore/src/EFCore.Analyzers/EFCore.Analyzers.csproj
Lines 30 to 31 in a204b62
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh, I was too slow to comment; you already merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry... Can slip it into the next thing (either you or me).
Any idea what 4.0.1 restricts us to in terms of VS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2022 (probably worth putting https://aka.ms/roslyn-packages in the comment)