Codist icon indicating copy to clipboard operation
Codist copied to clipboard
verified publisher icon wmjordan

Suggestion: Search for Optional Method Parameter Usage

Open fitdev opened this issue 1 year ago • 14 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.

fitdev avatar May 22 '24 07:05 fitdev

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?

wmjordan avatar May 22 '24 11:05 wmjordan

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.

fitdev avatar May 22 '24 12:05 fitdev

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. image

Situation 2 is really missing.

wmjordan avatar May 22 '24 12:05 wmjordan

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.

fitdev avatar May 22 '24 12:05 fitdev

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

wmjordan avatar May 22 '24 12:05 wmjordan

Well, after some time, I got this.

Here's a quick demonstration for the optional2 parameter: image

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

wmjordan avatar May 23 '24 09:05 wmjordan

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)

fitdev avatar May 23 '24 12:05 fitdev

Please test the new beta which includes this feature.

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

wmjordan avatar May 23 '24 13:05 wmjordan

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

wmjordan avatar May 23 '24 13:05 wmjordan

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)

fitdev avatar May 23 '24 21:05 fitdev

Please try the new beta.

wmjordan avatar May 24 '24 12:05 wmjordan

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)

fitdev avatar May 24 '24 13:05 fitdev

Please try the new beta.

wmjordan avatar May 25 '24 06:05 wmjordan

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!

fitdev avatar May 25 '24 07:05 fitdev

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

fitdev avatar May 26 '24 08:05 fitdev

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

wmjordan avatar May 26 '24 12:05 wmjordan

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.

fitdev avatar May 28 '24 07:05 fitdev

Please use the new beta which fixes the above problem.

wmjordan avatar May 28 '24 12:05 wmjordan

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;

fitdev avatar Jun 04 '24 20:06 fitdev

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

wmjordan avatar Jun 05 '24 03:06 wmjordan

The new beta has implemented finding argument assignments in attributes.

wmjordan avatar Jun 07 '24 06:06 wmjordan

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.

fitdev avatar Jun 07 '24 07:06 fitdev

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

wmjordan avatar Jun 07 '24 08:06 wmjordan

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

fitdev avatar Jun 07 '24 10:06 fitdev

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)

fitdev avatar Jun 26 '24 06:06 fitdev

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

wmjordan avatar Jun 26 '24 08:06 wmjordan