Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Redo pubternalyzer with Roslyn Operations #21996

Merged
merged 13 commits into from
Aug 16, 2020
Merged

Redo pubternalyzer with Roslyn Operations #21996

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cf3db52
Redo pubternalyzer with Roslyn Operations
roji Aug 8, 2020
8488397
Detect subclassing via a symbol action instead of via syntax
roji Aug 11, 2020
f44a136
Depend on Microsoft.CodeAnalysis.CSharp.Workspaces 3.7.0
roji Aug 11, 2020
28d090c
Fix symbol comparison method
roji Aug 11, 2020
e8dd3f5
Add analyzer release tracking
roji Aug 11, 2020
80a452e
Correct behavior for VB
roji Aug 11, 2020
b730e30
Implement some additional missing cases
roji Aug 11, 2020
3514b51
Suppress one more new warning
roji Aug 11, 2020
b27e8a7
Align Microsoft.CodeAnalysis versions across the solution
roji Aug 11, 2020
2fcd169
Address review comments
roji Aug 13, 2020
f8c7056
Analyze method declarations
roji Aug 13, 2020
68108d2
Address review comments
roji Aug 13, 2020
6d185f5
Suppress more newly-discovered violations
roji Aug 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion benchmark/EFCore.Benchmarks/EFCore.Benchmarks.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

<ItemGroup Condition=" '$(Configuration)' == 'Release' Or '$(Configuration)' == 'Debug' ">
<ProjectReference Include="..\..\src\EFCore.Relational\EFCore.Relational.csproj" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.3.1" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisVersion)" />
</ItemGroup>

<ItemGroup Condition=" '$(Configuration)' == 'Release22' Or '$(Configuration)' == 'Debug22' ">
Expand Down
3 changes: 3 additions & 0 deletions eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@
<MicrosoftExtensionsHostFactoryResolverSourcesVersion>5.0.0-rc.1.20402.3</MicrosoftExtensionsHostFactoryResolverSourcesVersion>
<MicrosoftExtensionsLoggingVersion>5.0.0-rc.1.20402.3</MicrosoftExtensionsLoggingVersion>
</PropertyGroup>
<PropertyGroup Label="Other dependencies">
<MicrosoftCodeAnalysisVersion>3.7.0</MicrosoftCodeAnalysisVersion>
</PropertyGroup>
</Project>
18 changes: 18 additions & 0 deletions src/EFCore.Analyzers/AnalyzerReleases.Shipped.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## Release 2.1.0

### New Rules
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
EF1000 | Usage | Warning | RawSqlStringInjectionDiagnosticAnalyzer, [Documentation](https://docs.microsoft.com/ef/core/querying/raw-sql)

## Release 3.0.0

### New Rules
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
EF1001 | Usage | Warning | InternalUsageDiagnosticAnalyzer

### Removed Rules
Rule ID | Category | Severity | Notes
--------|----------|----------|--------------------
EF1000 | Security | Disabled | RawSqlStringInjectionDiagnosticAnalyzer, [Documentation](https://docs.microsoft.com/ef/core/querying/raw-sql)
Empty file added src/EFCore.Analyzers/AnalyzerReleases.Unshipped.md
View file
Open in desktop
Empty file.
8 changes: 7 additions & 1 deletion src/EFCore.Analyzers/EFCore.Analyzers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

<ItemGroup>
<!-- NB: Version affects the minimum required Visual Studio version -->
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.3.1" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion)" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" Version="$(MicrosoftCodeAnalysisVersion)" PrivateAssets="all" />
<PackageReference Update="NETStandard.Library" PrivateAssets="all" />
</ItemGroup>

Expand All @@ -32,4 +33,9 @@
</ItemGroup>
</Target>

<ItemGroup>
<AdditionalFiles Include="AnalyzerReleases.Shipped.md" />
<AdditionalFiles Include="AnalyzerReleases.Unshipped.md" />
</ItemGroup>

</Project>
322 changes: 261 additions & 61 deletions src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions ...edQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,12 @@ private void AddInclude(
var includeMethod = navigation.IsCollection ? _includeCollectionMethodInfo : _includeReferenceMethodInfo;
var includingClrType = navigation.DeclaringEntityType.ClrType;
var relatedEntityClrType = navigation.TargetEntityType.ClrType;
#pragma warning disable EF1001 // Internal EF Core API usage.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you already know about these internal usages @AndriySvyryd, but just in case.

var entityEntryVariable = _trackQueryResults
? shaperBlock.Variables.Single(v => v.Type == typeof(InternalEntityEntry))
: (Expression)Expression.Constant(null, typeof(InternalEntityEntry));
#pragma warning restore EF1001 // Internal EF Core API usage.

var concreteEntityTypeVariable = shaperBlock.Variables.Single(v => v.Type == typeof(IEntityType));
var inverseNavigation = navigation.Inverse;
var fixup = GenerateFixup(
Expand Down
8 changes: 4 additions & 4 deletions .../Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.ReadItemQueryingEnumerable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,28 +254,28 @@ private bool TryGenerateIdFromKeys(IProperty idProperty, out object value)
{
var entityEntry = Activator.CreateInstance(_readItemExpression.EntityType.ClrType);

#pragma warning disable EF1001
#pragma warning disable EF1001 // Internal EF Core API usage.
var internalEntityEntry = new InternalEntityEntryFactory().Create(
_cosmosQueryContext.Context.GetDependencies().StateManager, _readItemExpression.EntityType, entityEntry);
#pragma warning restore EF1001
#pragma warning restore EF1001 // Internal EF Core API usage.

foreach (var keyProperty in _readItemExpression.EntityType.FindPrimaryKey().Properties)
{
var property = _readItemExpression.EntityType.FindProperty(keyProperty.Name);

if (TryGetParameterValue(property, out var parameterValue))
{
#pragma warning disable EF1001 // Internal EF Core API usage.
internalEntityEntry[property] = parameterValue;
#pragma warning restore EF1001 // Internal EF Core API usage.
}
}

#pragma warning disable EF1001 // Internal EF Core API usage.
internalEntityEntry.SetEntityState(EntityState.Added);
#pragma warning restore EF1001 // Internal EF Core API usage.

value = internalEntityEntry[idProperty];

#pragma warning disable EF1001 // Internal EF Core API usage.
internalEntityEntry.SetEntityState(EntityState.Detached);
#pragma warning restore EF1001 // Internal EF Core API usage.

Expand Down
4 changes: 4 additions & 0 deletions src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ public MigrationsModelDiffer(
{
Check.NotNull(typeMappingSource, nameof(typeMappingSource));
Check.NotNull(migrationsAnnotations, nameof(migrationsAnnotations));
#pragma warning disable EF1001 // Internal EF Core API usage.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @bricelam

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll defer to @AndriySvyryd on these since they're here for seed data

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #22063
Let's just say that the current approach is not as robust as it should be

Check.NotNull(changeDetector, nameof(changeDetector));
#pragma warning restore EF1001 // Internal EF Core API usage.
Check.NotNull(updateAdapterFactory, nameof(updateAdapterFactory));
Check.NotNull(commandBatchPreparerDependencies, nameof(commandBatchPreparerDependencies));

Expand Down Expand Up @@ -137,7 +139,9 @@ public MigrationsModelDiffer(
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
#pragma warning disable EF1001 // Internal EF Core API usage.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bricelam same

protected virtual IChangeDetector ChangeDetector { get; }
#pragma warning restore EF1001 // Internal EF Core API usage.

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
3 changes: 2 additions & 1 deletion test/EFCore.Analyzers.Tests/EFCore.Analyzers.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
<ProjectReference Include="..\..\src\EFCore.Analyzers\EFCore.Analyzers.csproj" />
<ProjectReference Include="..\..\src\EFCore.Relational\EFCore.Relational.csproj" />
<ProjectReference Include="..\EFCore.Tests\EFCore.Tests.csproj" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.3.1" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" Version="$(MicrosoftCodeAnalysisVersion)" />
<PackageReference Include="Microsoft.Extensions.DependencyModel" Version="$(MicrosoftExtensionsDependencyModelVersion)" />
</ItemGroup>

Expand Down
Loading