Add Set.singleton >>/|> Set.toList fix#476
Conversation
| , { call = | ||
| \checkInfo -> | ||
| AstHelpers.getSpecificUnreducedFnCall Fn.Set.singleton checkInfo.lookupTable checkInfo.firstArg | ||
| |> Maybe.map | ||
| (\setSingletonCall -> | ||
| Rule.errorWithFix | ||
| { message = qualifiedToString Fn.Set.toList ++ " on " ++ qualifiedToString Fn.Set.singleton ++ " will result in singleton list with that element" | ||
| , details = [ "You can replace this call by a singleton list with that element." ] | ||
| } | ||
| checkInfo.fnRange | ||
| [ Fix.replaceRangeBy checkInfo.parentRange ("[ " ++ checkInfo.extractSourceCode (Node.range setSingletonCall.firstArg) ++ " ]") ] | ||
| ) | ||
| , composition = | ||
| (onSpecificFnCallCanBeCombinedCheck | ||
| { args = [] | ||
| , earlierFn = Fn.Set.singleton | ||
| , combinedFn = Fn.List.singleton | ||
| } | ||
| ).composition | ||
| } | ||
| ] |
There was a problem hiding this comment.
Done this way to ensure that the rule give the literal list value for the call-version.
I guess one could use onSpecificFnCallCanBeCombinedCheck to suggest the List.singleton in both cases, and it will in the end just be literal i think?
Or if there is a cleaner option
There was a problem hiding this comment.
I agree that the call case should be handled separately.
As for onSpecificFnCallCanBeCombinedCheck, I think that deserves a rework in a separate PR anyway as it's often called with args = [] which it is pretty wasteful. That simpler helper could then also use a composition helper which we can use here instead :)
There was a problem hiding this comment.
as it's often called with args = [] which it is pretty wasteful
How is that wasteful? It's only allocating another field, which is pretty cheap I'd say. There is no allocation for [] since that's just a constant reference. I feel like I'm missing something.
There was a problem hiding this comment.
onSpecificFnCallCanBeCombinedCheck has multiple lets and a bunch of code specific to handling these args that could be skipped. I think you'll agree once I make the PR, especially because args = [] is used a whole bunch.
lue-bird
left a comment
lue-bird
left a comment
There was a problem hiding this comment.
looks nice, thank you!
| { message = qualifiedToString Fn.Set.toList ++ " on " ++ qualifiedToString Fn.Set.singleton ++ " will result in singleton list with that element" | ||
| , details = [ "You can replace this call by a singleton list with that element." ] | ||
| } |
There was a problem hiding this comment.
small possible improvements
-will result in singleton list with that element
+will result in a singleton list(omitting "that element" because we don't know what that refers to. Its probably better explained in the details)
-You can replace this call by a singleton list with that element.
+You can replace this call by a singleton list containing the element given to the ++ Fn.Set.singleton ++ call.(making it a bit more clear what "that element" is)
| , { call = | ||
| \checkInfo -> | ||
| AstHelpers.getSpecificUnreducedFnCall Fn.Set.singleton checkInfo.lookupTable checkInfo.firstArg | ||
| |> Maybe.map | ||
| (\setSingletonCall -> | ||
| Rule.errorWithFix | ||
| { message = qualifiedToString Fn.Set.toList ++ " on " ++ qualifiedToString Fn.Set.singleton ++ " will result in singleton list with that element" | ||
| , details = [ "You can replace this call by a singleton list with that element." ] | ||
| } | ||
| checkInfo.fnRange | ||
| [ Fix.replaceRangeBy checkInfo.parentRange ("[ " ++ checkInfo.extractSourceCode (Node.range setSingletonCall.firstArg) ++ " ]") ] | ||
| ) | ||
| , composition = | ||
| (onSpecificFnCallCanBeCombinedCheck | ||
| { args = [] | ||
| , earlierFn = Fn.Set.singleton | ||
| , combinedFn = Fn.List.singleton | ||
| } | ||
| ).composition | ||
| } | ||
| ] |
There was a problem hiding this comment.
I agree that the call case should be handled separately.
As for onSpecificFnCallCanBeCombinedCheck, I think that deserves a rework in a separate PR anyway as it's often called with args = [] which it is pretty wasteful. That simpler helper could then also use a composition helper which we can use here instead :)
| , details = [ "You can replace this call by a singleton list with that element." ] | ||
| } | ||
| checkInfo.fnRange | ||
| [ Fix.replaceRangeBy checkInfo.parentRange ("[ " ++ checkInfo.extractSourceCode (Node.range setSingletonCall.firstArg) ++ " ]") ] |
There was a problem hiding this comment.
usually fixes use keepOnlyAndSurroundWithFix:
keepOnlyAndSurroundWithFix
{ parentRange = checkInfo.parentRange
, keep = Node.range setSingletonCall.firstArg
, left = "[ "
, right = " ]"
}This is faster (and IMO extractSourceCode should be a last resort)
morteako
force-pushed
the
set-singleton-to-list
branch
from
c59aa98 to
1c0560a
Compare
| Rule.errorWithFix | ||
| { message = qualifiedToString Fn.Set.toList ++ " on " ++ qualifiedToString Fn.Set.singleton ++ " will result in singleton list with that element" | ||
| , details = [ "You can replace this call by a singleton list with that element." ] | ||
| { message = qualifiedToString Fn.Set.toList ++ " on " ++ qualifiedToString Fn.Set.singleton ++ " will result in singleton list" |
There was a problem hiding this comment.
-will result in singleton list
+will result in a singleton list
|
Thank you for the PR |
3 participants
Solves the set-part of #470
Set.toList (Set.singleton v)to[ v ]Set.singleton >> Set.toListtoList.singleton