Suggestion: Search for Optional Method Parameter Usage

Question

Suggestion: Search for Optional Method Parameter Usage

fitdev opened this issue 4 months ago · 26 comments

The Codist's Find Symbol functions are really useful. However one item is missing. It would be really nice to be able to find usage of optional method parameters by their callers, supporting both positional and named usages:

void Foo(int required, string optional = "", bool optional2 = false) { }

// Example when searching for `optional2` usage
void Callers() {
  Foo(25); // Not a match, because optionals are not specified
  Foo(25, "bar"); // Not a match, because it is not the searched optional
  Foo(25, "bar", true); //Match! We searched for optional2
  Foo(25, optional2: true); //Also Match! Named optional parameter we searched for.
}

It is often very needed to perform such a search when changing the default values of optional parameters or when refactoring code and changing method signatures.

Answer 1 · 2024-05-22T11:56:22.000Z

Well, I expand your situation to the following code snippet:

void Foo(int required, string optional = "", bool optional2 = false) { }

// Example when searching for `optional2` usage
void A() {
	Foo(25); // Not a match, because optionals are not specified
}
void B() {
	Foo(25, "bar"); // Not a match, because it is not the searched optional
}
void C() {
	Foo(25, "bar", true); //Match! We searched for optional2
}
void D() {
	Foo(25, optional2: true); //Also Match! Named optional parameter we searched for.
}

When changing the default values of optional2, should not we take care for the Foo called within A and B since the change of default optional2 might alter the behavior there, yet in C and D, where optional2 are explicitly assigned, the change would not affect the behavior there?

Answer 2 · 2024-05-22T12:01:51.000Z

When changing the default values of optional2, should not we take care for the Foo called within A and B since the change of default optional2 might alter the behavior there, yet in C and D, where optional2 are explicitly assigned, the change would not affect the behavior there?

You are right. I am sorry, I did not explain it correctly. Yes, both cases are actually valid and useful, and thus there should be 2 distinct searches:

When the value of optional parameter is explicitly assigned by the caller (caller uses that parameter): This is useful when we are refactoring and perhaps want to remove that parameter entirely - we need to know all the callers that use that optional parameter.
When the default value would be used (caller does not use the parameter): This is useful when we may need to change the default value of that parameter.

Answer 3 · 2024-05-22T12:36:04.000Z

For situation 1, if you want to remove a parameter, you can use the change signature refactoring provided by VS, and check the Preview changes checkbox, then potential affected changes will be listed.

Situation 2 is really missing.

Answer 4 · 2024-05-22T12:44:16.000Z

Thank you for the suggestion on case 1. Yes, I have tried it, but it is a pretty bad experience and prone to errors, as you may accidentally confirm signature change even though you did not want to.

I would much prefer to have a pinnable (like all Codist's popups) list of all found parameter usages with their assigned values provided - either direcvtly or in a tooltip. So I can then go one-by-one in that list and revioew such cases.

Answer 5 · 2024-05-22T12:46:16.000Z

Well, I understood.
I will see whether it is possible to do so.

Answer 6 · 2024-05-23T09:06:32.000Z

Well, after some time, I got this.

Here's a quick demonstration for the optional2 parameter:

For the optional parameter (I modified the test snippet):

Answer 7 · 2024-05-23T12:04:46.000Z

Wow! You are so fast! This looks great!!!

The only addition I would like to see is that perhaps it could be adjusted slightly with modifier keys (similar to how it is already done with other find symbol commands), such that there are options to display just: parameters with explicit value provided OR parameters that use default values (with the 3rd option being with no modifier keys that displays both cases)

Answer 8 · 2024-05-23T13:11:12.000Z

Please test the new beta which includes this feature.

The keyboard modifier alteration will be added to the next beta.

Answer 9 · 2024-05-23T13:11:54.000Z

BTW, I am feeling good for this feature as well :)

Answer 10 · 2024-05-23T21:18:13.000Z

Unfortunately VS crashed on the first try:

Application: devenv.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.ArgumentException
   at Microsoft.CodeAnalysis.CSharp.CSharpCompilation.GetSemanticModel(Microsoft.CodeAnalysis.SyntaxTree, Boolean)
   at Codist.CodeAnalysisHelper+<FindParameterAssignmentsAsync>d__88.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task)
   at Codist.Controls.SymbolCommands+<FindArgumentAssignmentsAsync>d__11.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task)
   at Codist.Controls.CSharpSymbolContextMenu+UIHost+<FindParameterAssignments>d__28.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at MS.Internal.CulturePreservingExecutionContext.Run(MS.Internal.CulturePreservingExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndWrapper.WndProc(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(System.Object)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(System.Windows.Threading.DispatcherPriority, System.TimeSpan, System.Delegate, System.Object, Int32)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr, Int32, IntPtr, IntPtr)

Answer 11 · 2024-05-24T12:27:07.000Z

Please try the new beta.

Answer 12 · 2024-05-24T13:25:02.000Z

Still crashes:

Application: devenv.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.ArgumentException
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.CheckSyntaxNode(Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetSymbolInfo(Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax, System.Threading.CancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetSymbolInfoFromNode(Microsoft.CodeAnalysis.SyntaxNode, System.Threading.CancellationToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpSemanticModel.GetSymbolInfoCore(Microsoft.CodeAnalysis.SyntaxNode, System.Threading.CancellationToken)
   at Codist.CodeAnalysisHelper+<FindParameterAssignmentsAsync>d__88.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task)
   at Codist.Controls.SymbolCommands+<FindArgumentAssignmentsAsync>d__11.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task)
   at Codist.Controls.CSharpSymbolContextMenu+UIHost+<FindParameterAssignments>d__28.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at MS.Internal.CulturePreservingExecutionContext.Run(MS.Internal.CulturePreservingExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndWrapper.WndProc(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(System.Object)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(System.Windows.Threading.DispatcherPriority, System.TimeSpan, System.Delegate, System.Object, Int32)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr, Int32, IntPtr, IntPtr)

Answer 13 · 2024-05-25T06:46:36.000Z

Please try the new beta.

Answer 14 · 2024-05-25T07:28:01.000Z

Now it did not crash :) I will keep on using it and let you know if I encounter any issues. Thank you for such a great work!

I also really like how you aligned the call sites that all belong to the same method! Really useful!

Answer 15 · 2024-05-26T08:17:20.000Z

So far seems to be working great! Thanks so much for all your work!

Answer 16 · 2024-05-26T12:08:07.000Z

Thank you very much for sharing your idea and the feedback.

Answer 17 · 2024-05-28T07:31:12.000Z

Just noticed that when searching for default assignments only (via Shift), it incorrectly shows the same list as when searching for all assignments, but lists the value as default for all of them (also incorrectly). Explicit assignments work fine.

Answer 18 · 2024-05-28T12:16:09.000Z

Please use the new beta which fixes the above problem.

Answer 19 · 2024-06-04T20:23:41.000Z

Just found another issue: Apparently it does not find any usages of a parameter if that usage is in Attribute constructor:

public sealed class MyAttribute : Attribute {
  public MyAttribute(string testarg = "") { } // Usage of testarg is not found at all
}

[MyAttribute("test")]
bool Foo;

Answer 20 · 2024-06-05T03:52:53.000Z

No, it does not.
It takes extra code to cope with that situation.

Answer 21 · 2024-06-07T06:49:44.000Z

The new beta has implemented finding argument assignments in attributes.

Answer 22 · 2024-06-07T07:08:34.000Z

Thanks so much for your work! This actually is pretty important for me as I have lots of cases where enums have attributes with complex constructors, so it is quite useful!

However there is a minor issue with the latest version. While it finds all instances of parameter usage, the value that is displayed in the list on the right is often incorrect.

In particular, if the parameter is optional and is not provided at the call site, but, because the "call site" is in the attributes, the [Attr(requiredarg, PropertyName = 23)] property syntax can also be used.

So, given:

public sealed class MyAttr : Attribute {

public MyAttr(string requiredarg, bool optionalarg = false) { }

public int PropertyName {get; set;}

}

Enum Foo {
[MyAttr("foostring", PropertyName = 77)]
MyValue = 0,
}

The usages of optionalarg would find Foo.MyValue correctly, but would report 77 as the value, which is incorrect (it should be the default = 0). Presumably this is because PropertyName is in the same "slot" as the optional argument would be in. had it been provided.

Answer 23 · 2024-06-07T08:54:56.000Z

You are right. The 77 is in the same "slot".
Please download the new beta.

Answer 24 · 2024-06-07T10:16:22.000Z

Thank you for a quick fix! This works great now!

Answer 25 · 2024-06-26T06:25:54.000Z

Got another VS crash when tried using the functionality:

Application: devenv.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.NullReferenceException
   at Codist.CodeAnalysisHelper.AreEqual(Microsoft.CodeAnalysis.ITypeSymbol, Microsoft.CodeAnalysis.ITypeSymbol, Boolean)
   at Codist.CodeAnalysisHelper+<FindParameterAssignmentsAsync>d__88.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task)
   at Codist.Controls.SymbolCommands+<FindParameterAssignmentsAsync>d__11.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(System.Threading.Tasks.Task)
   at Codist.Controls.CSharpSymbolContextMenu+UIHost+<FindParameterAssignments>d__28.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at MS.Internal.CulturePreservingExecutionContext.Run(MS.Internal.CulturePreservingExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndWrapper.WndProc(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(System.Object)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(System.Windows.Threading.DispatcherPriority, System.TimeSpan, System.Delegate, System.Object, Int32)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr, Int32, IntPtr, IntPtr)

Answer 26 · 2024-06-26T08:26:50.000Z

It is a bug.
I've just published a new version 7.9.1 to the Marketplace to fix this.