Simplifier options by tmat · Pull Request #60174 · dotnet/roslyn

tmat · 2022-03-14T23:55:49Z

Add (CSharp|VisualBasic)SimplifierOptions containing all code style options used by simplifiers.

The first commit introduces SimplifierOptions and threads them through.

Since these options are editorconfig options we also need to pass around "fallback options" that are used when a simplifier option is not specified in the editorconfig files applicable to the given document. The fallback options are only available in the IDE - they are stored in the registry and accessed via IGlobalOptionService. In this PR fallback options are not required since we still fall back from DocumentOptions to the solution options. To limit the size of the PR some call sites that create options pass null fallback options -- these will be updated in a follow up PR.

Since the set of available simplifier options is language specific, we can't use a single SimplifierOptionsStorage extension method defined for IGlobalOptionService as it does not know how to construct the derived option types. We introduce ISimplifierOptionStorage language service for that purpose.

Recommended to review commit by commit. The first commit already reviewed by @ryzngard.

ryzngard

src/Workspaces/VisualBasicTest/CodeGeneration/AddImportsTests.vb

sharwell · 2022-04-05T17:41:35Z

Integration tests showing gold bar

Feature 'Diagnostic analyzer runner' is currently unavailable due to an internal error.
MessagePack.MessagePackSerializationException : Error writing JSON RPC Message: MessagePackSerializationException: Failed to serialize Microsoft.CodeAnalysis.Diagnostics.DiagnosticArguments value. ---> MessagePack.MessagePackSerializationException : Failed to serialize Microsoft.CodeAnalysis.Diagnostics.DiagnosticArguments value. ---> MessagePack.FormatterNotRegisteredException : Microsoft.CodeAnalysis.Simplification.SimplifierOptions is not registered in resolver: StreamJsonRpc.MessagePackFormatter+ResolverWrapper
   at MessagePack.FormatterResolverExtensions.Throw(Type t,IFormatterResolver resolver)
   at MessagePack.FormatterResolverExtensions.GetFormatterWithVerify[T](IFormatterResolver resolver)
   at Serialize(Byte[][] ,Object[] ,MessagePackWriter& ,IdeAnalyzerOptions ,MessagePackSerializerOptions )
   at MessagePack.Internal.AnonymousSerializableFormatter`1.Serialize(MessagePackWriter& writer,T value,MessagePackSerializerOptions options)
   at Serialize(Byte[][] ,Object[] ,MessagePackWriter& ,DiagnosticArguments ,MessagePackSerializerOptions )
   at MessagePack.Internal.AnonymousSerializableFormatter`1.Serialize(MessagePackWriter& writer,T value,MessagePackSerializerOptions options)
   at MessagePack.MessagePackSerializer.Serialize[T](MessagePackWriter& writer,T value,MessagePackSerializerOptions options)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
   --- End of inner exception stack trace ---
   at MessagePack.MessagePackSerializer.Serialize[T](MessagePackWriter& writer,T value,MessagePackSerializerOptions options)
   at StreamJsonRpc.MessagePackFormatter.JsonRpcRequestFormatter.Serialize(MessagePackWriter& writer,JsonRpcRequest value,MessagePackSerializerOptions options)
   at StreamJsonRpc.MessagePackFormatter.JsonRpcMessageFormatter.Serialize(MessagePackWriter& writer,JsonRpcMessage value,MessagePackSerializerOptions options)
   at StreamJsonRpc.MessagePackFormatter.Serialize(IBufferWriter`1 contentBuffer,JsonRpcMessage message)
   --- End of inner exception stack trace ---
   at StreamJsonRpc.MessagePackFormatter.Serialize(IBufferWriter`1 contentBuffer,JsonRpcMessage message)
   at StreamJsonRpc.LengthHeaderMessageHandler.Write(JsonRpcMessage content,CancellationToken cancellationToken)
   at StreamJsonRpc.PipeMessageHandler.WriteCoreAsync(JsonRpcMessage content,CancellationToken cancellationToken)
   at async StreamJsonRpc.MessageHandlerBase.WriteAsync(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async StreamJsonRpc.JsonRpc.SendAsync(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async StreamJsonRpc.JsonRpc.InvokeCoreAsync(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async StreamJsonRpc.JsonRpc.InvokeCoreAsync[TResult](<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.CodeAnalysis.Remote.BrokeredServiceConnection`1.TryInvokeAsync[TService,TResult](<Unknown Parameters>)

src/EditorFeatures/CSharp/Simplification/CSharpGlobalIdeOptionsProvider.cs

CyrusNajmabadi · 2022-04-14T18:39:47Z

src/EditorFeatures/Test2/Simplification/AbstractSimplificationTests.vb

            Else
                simplifiedDocument = Await Simplifier.ReduceAsync(document, Simplifier.Annotation, optionSet)
            End If
+#Enable Warning RS0030


could just update this to use the new apis right?

src/EditorFeatures/Test2/Simplification/SimplifierAPITests.vb

src/EditorFeatures/VisualBasic/Simplification/VisualBasicSimplifierOptionsStorage.vb

...ixes/Suppression/AbstractSuppressionCodeFixProvider.GlobalSuppressMessageFixAllCodeAction.cs

src/Features/Core/Portable/EncapsulateField/EncapsulateFieldOptions.cs

CyrusNajmabadi · 2022-04-14T18:45:49Z

src/Features/Core/Portable/MetadataAsSource/MetadataAsSourceOptions.cs


        public MetadataAsSourceOptions()
-            : this(NavigateToDecompiledSources: true)
+            : this(SimplifierOptions: null)


aside: i'm not really a fan of using structs here primarily because i find having to reason about no-param constructors (vs, all optional constructors) so painful. Consider moving these to classes.

src/Features/Core/Portable/Options/IGlobalIdeOptionsProvider.cs

...s/CSharp/Portable/Simplification/Reducers/AbstractCSharpReducer.AbstractReductionRewriter.cs

CyrusNajmabadi · 2022-04-14T18:51:49Z

...s/CSharp/Portable/Simplification/Reducers/AbstractCSharpReducer.AbstractReductionRewriter.cs

+            {
+                Contract.ThrowIfNull(ParseOptions);
+                Debug.Assert(Options != null);
+                Debug.Assert(SemanticModel != null);


just use Contract.ThrowIfNull for all of htem :)

src/Workspaces/CSharp/Portable/Simplification/Reducers/AbstractCSharpReducer.cs

src/Workspaces/Core/Portable/ChangeNamespace/ChangeNamespaceOptions.cs

src/Workspaces/CoreTest/Remote/ServiceDescriptorTests.cs

...haredUtilitiesAndExtensions/Workspace/Core/LanguageServices/AddImports/IAddImportsService.cs

…res/required-members * upstream/main: (66 commits) Fix #55183: Add SymbolVisitor<TArgument, TResult> (#56530) Simplifier options (#60174) Remove duplicated asset Do not try to refcount solution syncing when communicating with OOP Delay symbol-search index updating until solution is fully loaded. add more miscellaneous tests for checked operators (#60727) Support checked operators in explicit interface implementation (#60715) Avoid formatting diagnostics with raw strings (#60655) Make heading levels for warning waves documentation consistent (#60721) Clean up IDiagnosticService extension methods Remove #nullable enable Add integration test to flag MEF composition breaks Generate static abstract interface members correctly (#60618) Merge release/dev17.2 to main (#60682) Fix FAR on checked operators (#60698) Add implement interface support for checked operators and cast operators (#60719) Update csc.dll path in launch.json (#60663) Grab bag of UTF8 string support in IDE features (#60599) Allow code actions to retrieve options for any language (#60697) Fix flaky VSTypeScriptHandlerTests (#60706) ...

…o setsrequiredmembers * upstream/features/required-members: (66 commits) Fix dotnet#55183: Add SymbolVisitor<TArgument, TResult> (dotnet#56530) Simplifier options (dotnet#60174) Remove duplicated asset Do not try to refcount solution syncing when communicating with OOP Delay symbol-search index updating until solution is fully loaded. add more miscellaneous tests for checked operators (dotnet#60727) Support checked operators in explicit interface implementation (dotnet#60715) Avoid formatting diagnostics with raw strings (dotnet#60655) Make heading levels for warning waves documentation consistent (dotnet#60721) Clean up IDiagnosticService extension methods Remove #nullable enable Add integration test to flag MEF composition breaks Generate static abstract interface members correctly (dotnet#60618) Merge release/dev17.2 to main (dotnet#60682) Fix FAR on checked operators (dotnet#60698) Add implement interface support for checked operators and cast operators (dotnet#60719) Update csc.dll path in launch.json (dotnet#60663) Grab bag of UTF8 string support in IDE features (dotnet#60599) Allow code actions to retrieve options for any language (dotnet#60697) Fix flaky VSTypeScriptHandlerTests (dotnet#60706) ...

ghost added the Area-IDE label Mar 14, 2022

tmat force-pushed the SimplifierOptions branch 2 times, most recently from 2047746 to a930435 Compare March 16, 2022 00:37

tmat marked this pull request as ready for review March 16, 2022 00:37

tmat requested a review from a team as a code owner March 16, 2022 00:37

ryzngard approved these changes Mar 16, 2022

View reviewed changes

src/Workspaces/VisualBasicTest/CodeGeneration/AddImportsTests.vb Outdated Show resolved Hide resolved

tmat marked this pull request as draft March 16, 2022 14:35

This was referenced Mar 18, 2022

Failure in CSharpCodeActions.GFUFuzzyMatchAfterRenameTrackingAndAfterGenerateType #57423

Closed

FixAllOccurrencesIgnoresGeneratedCode is flaky #56625

Closed

tmat force-pushed the SimplifierOptions branch from 2146869 to 6bbc207 Compare April 4, 2022 21:37

tmat force-pushed the SimplifierOptions branch 4 times, most recently from 579ac53 to 5a8b5e2 Compare April 12, 2022 04:37

tmat force-pushed the SimplifierOptions branch 5 times, most recently from 003608b to 21d8d35 Compare April 14, 2022 00:03

tmat added 8 commits April 13, 2022 22:23

SimplifierOptions

7cd04c1

Public option tests

a314596

Serializability

aff0f43

Fallback simplifier options for analyzers

b6e09f4

Null annotate

992732e

Fallback simplifier options for code actions

8f7f763

Document extension

366135b

Avoid calling Reducer APIs that take OptionSet; CodeCleanupOptions

0f04198

tmat force-pushed the SimplifierOptions branch from 28295c8 to 0f04198 Compare April 14, 2022 05:23

Fix test

574e363