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

Run the internal code usage analyzer on the providers #15393

Closed
roji opened this issue Apr 17, 2019 · 7 comments · Fixed by #20567
Closed

Run the internal code usage analyzer on the providers #15393

roji opened this issue Apr 17, 2019 · 7 comments · Fixed by #20567
Labels
area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported poachable punted-for-3.0 type-enhancement
Milestone
5.0.0

Comments

@roji
Copy link
Member

roji commented Apr 17, 2019

We have an analyzer that detects usage of internal code, but it doesn't run on the EF Core providers in the solution since analyzers don't work across project references.

Look into running the analyzer from a test instead, for the providers only. This could also be useful in the functional test suite for other providers.
@roji roji mentioned this issue Apr 19, 2019
Closed
@ajcvickers ajcvickers added this to the 3.0.0 milestone Apr 19, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog May 10, 2019
@AndriySvyryd AndriySvyryd self-assigned this Aug 23, 2019
@bricelam bricelam removed their assignment Nov 5, 2019
@ajcvickers
Copy link
Member

We need some way to do this automatically, but does not have to be analyzer.

Muppets added a commit to Muppets/EntityFrameworkCore that referenced this issue Mar 9, 2020
@Muppets
Fixes dotnet#15393 Added test to check providers don't use internal APIs
0296dab
@Muppets Muppets mentioned this issue Mar 9, 2020
Closed
@bricelam
Copy link
Contributor

bricelam commented Mar 16, 2020 •
edited
Loading

Project Violations Notes
Cosmos 27 Covered by #16707
Design 1,214 Don't enable
InMemory 17
Proxies 8 Known
Relational 275 Smit to investigate
Sqlite 2
Sqlite.NTS 2
SqlServer 4
SqlServer.NTS 0

@bricelam
Copy link
Contributor

Looks like we can use a project reference:

<ProjectReference Include="..\EFCore.Analyzers\EFCore.Analyzers.csproj"
                  ReferenceOutputAssembly="False"
                  OutputItemType="Analyzer" />

Some people have reported issues (e.g. having to unload and reload projects) but we can try it and see how annoying it is.

@ajcvickers
Copy link
Member

Removing from Backlog to remember to discuss on Friday.

@ajcvickers ajcvickers removed this from the Backlog milestone Mar 16, 2020
@ajcvickers
Copy link
Member

Notes from discussion:

  • @smitpatel to investigate the relational uses.
    • It may be correct to allow relational to use internal references, as long as these internal uses are not blocking things needed by non-relational providers.
    • We should follow up with other non-relational providers to see what they are using
  • For 5.0, we need to check/fix the other uses
    • Some of them are intentional
    • We should run the analyzer against providers even if we have to suppress in some cases
    • SqlServer uses PropertyBaseExtensions, ConventionAnnotation & RelationalExpressionExtensions
    • Sqlite uses TypeExtensions
    • Sqlite.NTS uses SqliteOptionsExtension

See also #16707 for Cosmos

@ajcvickers ajcvickers added this to the 5.0.0 milestone Mar 20, 2020
smitpatel added a commit that referenced this issue Mar 24, 2020
@smitpatel
Make ConventionAnnotatable public
4f9befe 
Part of #15393
@smitpatel smitpatel mentioned this issue Mar 24, 2020
Merged
smitpatel added a commit that referenced this issue Mar 24, 2020
@smitpatel
Suppress internal warnings for DatabaseFacadeDependencies
1aa34e4 
Part of #15393
@smitpatel smitpatel mentioned this issue Mar 24, 2020
Merged
smitpatel added a commit that referenced this issue Mar 24, 2020
@smitpatel
Suppress internal warning in TableNameFromDbSetConvention
febb301 
Part of #15393
@smitpatel smitpatel mentioned this issue Mar 24, 2020
Closed
@smitpatel
Copy link
Member

Apart from few issues/PR I filed, comprehensive report.

ForeignKeyConstraint.cs: ForeignKeyComparer 
TableIndex.cs: IndexComparer 
UniqueConstraint.cs: KeyComparer 
MigrationsSqlGenerator.cs: SemanticVersionComparer 
FromSqlParameterApplyingExpressionVisitor.cs: ReferenceEqualityComparer 
RelationalValueGeneratorSelector.cs: TemporaryNumberValueGeneratorFactory 
RelationalCommandBuilder.cs: IndentedStringBuilder 
TableAliasUniquifyingExpressionVisitor.cs: ReferenceEqualityComparer 
KeyValueIndexFactorySource.cs: IdentityMapFactoryFactoryBase 
RelationalValueGeneratorSelector.cs: TemporaryDateTimeValueGenerator 
RelationalValueGeneratorSelector.cs: TemporaryDateTimeOffsetValueGenerator 
RelationalValueGeneratorSelector.cs: StringValueGenerator 
RelationalValueGeneratorSelector.cs: BinaryValueGenerator 
ModificationCommand.cs: NonCapturingLazyInitializer 
KeyValueIndexFactory.cs: IPrincipalKeyValueFactory<TKey> 
KeyValueIndexFactory.cs: IDependentKeyValueFactory<TKey> 
KeyValueIndexFactory.cs: ForeignKeyExtensions 
KeyValueIndexFactorySource.cs: KeyExtensions 
TableMappingBaseComparer.cs: EntityTypePathComparer 
MigrationsModelDiffer.cs: Multigraph<,> 
ModificationCommand.cs: InternalEntityEntry 
ModificationCommandComparer.cs: PropertyBaseExtensions 
CommandBatchPreparer.cs: Multigraph<,> 
Migrator.cs: IndentedStringBuilder 
CommandBatchPreparer.cs: Graph<,> 
CommandBatchPreparer.cs: IndexExtensions 
CommandBatchPreparer.cs: IDependentKeyValueFactory<object[]> 
CommandBatchPreparer.cs: INullableValueFactory<object[]> 
MigrationsSqlGenerator.cs: CoreAnnotationNames 
MigrationsModelDiffer.cs: EntityTypeExtensions 
ColumnMappingBaseComparer.cs: EntityTypePathComparer 
RelationalTypeMappingSource.cs: PropertyExtensions 
RelationalTypeMappingInfo.cs: PropertyExtensions 
RelationalExecutionStrategyFactory.cs: NoopExecutionStrategy 
RelationalModel.cs: DebugView 
RelationalModel.cs: ForeignKeyComparer 
RelationalModel.cs: EntityTypePathComparer

smitpatel added a commit that referenced this issue Mar 25, 2020
@smitpatel
Suppress internal warnings for DatabaseFacadeDependencies
40b2400 
Part of #15393
smitpatel added a commit that referenced this issue Mar 25, 2020
@smitpatel
Cleanup Relational for internal API usage
7614da5 
Part of #15393

Changes:
- Suppress DatabaseFacadeDependencies related errors. Optimization which no otherprovider should need.
- Suppress errors with DebugView & metadata comparers in relationl model.
- Suppress NoopExecutionStrategy in RelationalExecutionStrategyFactory
- Suppress FindDeclaredPrimaryKey in MigrationModelDiffer
- Suppress errors related to valuefactories and InternalEntityEntry in update pipeline
- Suppress errors related to temporary value generator in RelationalValueGeneratorSelector

- Move SemanticVersionComparer to Relational
- Move DbSetFinderExtensions, DbSetProperty, ConventionAnnotatable, Graph, IndentedStringBuilder to .Infrastructure
- Move Multigraph, ReferenceEqualityComparer to shared internal

- Remove DbSetProperty.IsKeyless - dead code
smitpatel added a commit that referenced this issue Mar 25, 2020
@smitpatel
Cleanup Relational for internal API usage
fa2ff1e 
Part of #15393

Changes:
- Suppress DatabaseFacadeDependencies related errors. Optimization which no otherprovider should need.
- Suppress errors with DebugView & metadata comparers in relationl model.
- Suppress NoopExecutionStrategy in RelationalExecutionStrategyFactory
- Suppress FindDeclaredPrimaryKey in MigrationModelDiffer
- Suppress errors related to valuefactories and InternalEntityEntry in update pipeline
- Suppress errors related to temporary value generator in RelationalValueGeneratorSelector

- Move SemanticVersionComparer to Relational
- Move DbSetFinderExtensions, DbSetProperty, ConventionAnnotatable, Graph, IndentedStringBuilder to .Infrastructure
- Move Multigraph, ReferenceEqualityComparer to shared internal

- Remove DbSetProperty.IsKeyless - dead code
@smitpatel smitpatel removed their assignment Mar 25, 2020
@smitpatel
Copy link
Member

Enabled in relational with suppression in place.

ajcvickers added a commit that referenced this issue Apr 7, 2020
@ajcvickers
Enable internal code usage analyzer on the providers
a24136a 
Fixes #15393
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 7, 2020
@ajcvickers ajcvickers mentioned this issue Apr 7, 2020
Merged
ajcvickers added a commit that referenced this issue Apr 7, 2020
@ajcvickers
Enable internal code usage analyzer on the providers
3f6f4e7 
Fixes #15393
ajcvickers added a commit that referenced this issue Apr 14, 2020
@ajcvickers
Enable internal code usage analyzer on the providers
cab5f4f 
Fixes #15393
ajcvickers added a commit that referenced this issue Apr 16, 2020
@ajcvickers
Enable internal code usage analyzer on the providers
2ca6a19 
Fixes #15393
ajcvickers added a commit that referenced this issue Apr 17, 2020
@ajcvickers
Enable internal code usage analyzer on the providers
1fc5904 
Fixes #15393
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview4 Apr 20, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview4, 5.0.0 Nov 7, 2020
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-test closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported poachable punted-for-3.0 type-enhancement
Projects
None yet
Milestone
5.0.0
5 participants
@bricelam @ajcvickers @smitpatel @roji @AndriySvyryd