microsoft / qsharp

Azure Quantum Development Kit, including the Q# programming language, resource estimator, and Quantum Katas
https://microsoft.github.io/qsharp/
MIT License
460 stars 92 forks source link

Linter suggests to replace lambda operation expression with lambda function expression #2039

Open msoeken opened 1 week ago

msoeken commented 1 week ago

Describe the bug

Linter suggests to replace operation lambda expression with function lambda expression, even though this would not work, because the callable signature requires an operation.

To Reproduce

https://microsoft.github.io/qsharp/?code=H4sIAAAAAAAAE22QT0vDQBDF7%252FkUj542ElMKnhoqiFUU%252FIO24qHksE3XZKDZze5OLEXy3WXTYIt0LjPMm9%252BDeVQ3xjEWvElvnJN7n15kER2Xc5KlNp6p6JXINMpJJqPxSVwt2lpYjyne2jXxKk8gi14cNphd40MTx5j2HT8RANzLgoU2jEd%252FVze8F9bHCUbWo5A67NcKKgijOIt6YkdckR7wUF%252FGgUAakzR9UrrkKpjgEpOTo1C3L69LYf2KgpYnCGMeZ383XT91kE2z3Z%252Bwh0fEUtI2OA9EF3WnGTxL0uLfc61XsB6zIZOr%252FEAe40pgQy7ztm7eVUmelRMrm8dxhvEYO2d0OYWvTLvdIIRR0reCxE46Tbo8b%252FYg7MAXxjlV8BQbo%252Fw5vot%252BAVKAZZX0AQAA&profile=unrestricted

Expected behavior

No linter warning should appear.

Additional context

This is similar to #1661

Manvi-Agrawal commented 6 days ago

@msoeken I think this bug is interesting edge case which might be tricky to tackle. I think this is arising due to function being promoted to operation because of all able conversation rules on Q#. All functions are operations. But not all operations are functions.

The example involves a function invoking DumpOperation function being passed as lambda argument. According to my understanding DumpOperation does not work on real hardware. I understand that it might be a little inconvenient for simulation purposes and might require refactoring. Do you have some other practical example(s) in mind for real hardware?

What if somebody decides to create a operation CustomDumpOperation that only invokes DumpOperation function. And then passed CustomDumpOperation to a operation accepting operation as argument. Then it seems we shouldn't flag it as warning, am I correct? I wonder this might lead to a snow ball effect, encouraging more operations.

What if the CustomDumpOperation is required to be declared as an operation for only few operation calls. And it works perfectly fine as a function for other cases. Is the suggestion to gravitate towards allowing CustomDumpOperation as a operation. I am afraid this kind of defeats the purpose of this lint rule.

Given the fact that this is a lint rule which can be turned off, I believe the current implementation is good enough for regular usage. Some other potential ways to tackle this might be to offer an improved linting functionality: (I) like inclusion/exclusion rules for file paths for each lint, (ii) having an allow list for a function to be treated as a Operation. So, in this case we could instruct QDK to treat DumpOperation as operation so that we don't get the warning. Thoughts?

PS: sent from mobile, not responsible for autocorrect errors