Follow up items after PR 41363

[mavasani]

During the design meeting discussion on https://github.com/dotnet/roslyn/pull/41363, we decided to go ahead with the current approach in the PR with the following follow-up items:

• [x] [Implemented with 41687 and 41768] De-dupe analyzers and code fixes in IDE Features layer and Code Style layer inside the diagnostic analyzer and code fix engines. Currently, both the copies execute and we also see duplicate diagnostics being reported.
• [ ] Remove MEF based discovery of language services. Replace it with an explicit mapping from each language service to the type implementing it. We also need to design how external clients, similar to CodeStyle, can be supported, which means we may not be able to explicitly hardcode the assembly/types that implement the language services.
• [x] Remove duplicated resources from CodeStyle and Workspaces/Features layer with a shared resource file. Need to wait on @tmat's change that simplifies resource generation in Roslyn.sln.
• [ ] Revert changes required to get existing IDE analyzer/fixer tests working in CodeStyle layer once all the tests are switched to Microsoft.CodeAnalysis.Testing:
  • [ ] Test framework files linked into CodeStyle.UnitTests projects
  • [ ] IVTs added from Roslyn test utilities and Workspaces to CodeStyle.UnitTests projects
  • [ ] Project references added to test utilities project
• [ ] Move all our analyzers and fixers to work completely based on AnalyzerConfigOptions instead of OptionSet. This involves adding support for creating and passing a fallback .editorconfig/AnalyzerConfigOptions from Tools Options settings into analysis context.
• [ ] Remove use of GetDocumentOptionSetAsync API in the shared layer, which is currently used to get fallback Workspace options. Likely the work in prior bullet item will automatically take care of this, but adding an explicit cleanup item here so we validate that.
• [ ] Remove the TODOs added for code moved to shared layer which uses very recently added compiler APIs, which are not yet available in CodeStyle layer. For example, see this comment.
• [x] [Implemented with 42323] Remove all the #if CODE_STYLE from the shared projects. All of them should be possible to remove trivially, except the ones related to public Options related types (OptionSet, IOption, CodeStyleOption, CodeStyleOptions, NotificationOptions, etc.), which are public APIs in Workspaces layer but also need by the CodeStyle analyzers which do not have access to Workspaces.
• [ ] We have lot more scope of refactoring the existing extension methods and utilities in Workspaces layer and move them down into the shared layers. There is scope even to move lot of code in WorkspaceExtensions layer that is currently used by code fixes, but not specific to Workspaces itself, to be moved down to CompilerExtensions layer. This will likely be a continuous refactoring work item.
• [ ] CodeAction.DocumentChangeAction is used in IDE code fixes, and was replaced with a new type CustomCodeActions in shared layer. Do we want to get rid of the CustomCodeActions types completely? Do we want to avoid them just in CodeStyle layer but still use them for analyzers linked into Features? Latter will introduce #if CODE_STYLE clutter in each code fix file, which seems undesirable.
• [ ] Cleanup the API signature for SytnaxEditorBasedCodeFixProvider.IncludeDiagnosticDuringFixAll as per https://github.com/dotnet/roslyn/pull/41510#discussion_r377391657
• [ ] Remove AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer.IsRegularCommentOrDocComment and directly call into syntax facts service once it is available in shared analyzer layer.
• [ ] Consider deleting IGeneratedCodeRecognitionService.IsGeneratedCode and moving all existing callers to IGeneratedCodeRecognitionService.IsGeneratedCodeAsync
• [ ] Rationalize use of SyntaxGenerator.GetGenerator(document) versus document.GetRequiredLanguageService<SyntaxGenerator>() in our code base.
• [x] [Addressed with 43367] Revert #if CODE_STYLE directives added to product source and tests due for the scenario where certain new Nullability APIs are not yet available in CodeStyle layer. This might mean either moving the CodeStyle layer to new Microsoft.CodeAnalysis API OR adding reflection based lightup support in CodeStyle layer.
• [ ] Revert #if CODE_STYLE directives related to ITypeSymbol.IsNativeIntegerType (not yet available in CodeStyle layer). See PR https://github.com/dotnet/roslyn/issues/41462
• [ ] Consider porting ISemanticFactsService to shared CompilerExtensions project so they can be used in our analyzers in the shared layer. See https://github.com/dotnet/roslyn/pull/42264#discussion_r390999980
• [ ] Nullability related changes mentioned in https://github.com/dotnet/roslyn/pull/42323#discussion_r392717832 and https://github.com/dotnet/roslyn/pull/42323#discussion_r392720810
• [ ] Update nullability of OptionSet in EditorConfigStorageLocation2 to allow null values. See https://github.com/dotnet/roslyn/pull/42323#discussion_r392730378
• [ ] Move all the IDE tests that use options to pass in IOptionsCollection instead of IDictionary<OptionKey, object. See https://github.com/dotnet/roslyn/pull/42323#discussion_r395386979

[mavasani]

Tag @tmat @sharwell @jasonmalinowski - let me know if I forgot any other follow-up cleanup items that we came up with in the meeting.

[mavasani]

@sharwell @CyrusNajmabadi can we bring this to Monday's design meeting to review specifically the last item?

Revert #if CODE_STYLE directives added to product source and tests due for the scenario where certain new Nullability APIs are not yet available in CodeStyle layer. This might mean either moving the CodeStyle layer to new Microsoft.CodeAnalysis API OR adding reflection based lightup support in CodeStyle layer.

It should hopefully be a quick discussion and would bring back bunch of missing functionality in CodeStyle layer.

[mavasani]

Marking for next meeting - we need to really decide if we are going to use lightup/reflection in CodeStyle layer or become lenient in moving CodeStyle layer to newer Microsoft.CodeAnalysis to be able to access newly added APIs.

[CyrusNajmabadi]

Closing out as we haven't done anything with this in 4 years.