Add new rule CSharpFriendlyAsyncOverload #533

su8898 · 2021-12-20T03:40:04Z

Fixes #517

knocte · 2021-12-20T03:48:00Z

docs/content/how-tos/rules/FL0078.md

+
+# FriendlyAsyncOverload (FL0078)
+
+*Introduced in `0.20.3`*


that ship has already sailed, please update version number

knocte · 2021-12-20T03:48:45Z

docs/content/how-tos/rules/FL0078.md

+
+## Rationale
+
+Exposing public F#-async APIs in a C#-friendly manner for better C# interoperability.


let's say "async APIs" here, not "F#-async APIs"

knocte · 2021-12-20T03:49:13Z

docs/content/how-tos/rules/FL0078.md

+
+## How To Fix
+
+Add an `Async` version of the API that returns a `Task<'T>`


Add an Async-suffixed*

knocte · 2021-12-20T03:49:39Z

src/FSharpLint.Core/Rules/Identifiers.fs

 let FavourTypedIgnore = identifier 70
 let CyclomaticComplexity = identifier 71
 let FailwithBadUsage = identifier 72
+let FriendlyAsyncOverload = identifier 77


this looks like the PR needs a rebase

knocte · 2021-12-20T03:50:05Z

src/FSharpLint.Core/Text.resx

    <value>Bad usage of failwith: {0}.</value>
  </data>
+  <data name="RulesFriendlyAsyncOverload" xml:space="preserve">
+    <value>Consider using a friendly async overload for {0}.</value>


a C#-friendly* async overload

knocte · 2021-12-20T03:52:25Z

src/FSharpLint.Core/fsharplint.json

    "invalidArgWithTwoArguments": { "enabled": true },
    "failwithfWithArgumentsMatchingFormatString": { "enabled": true },
    "failwithBadUsage": { "enabled": true },
+    "friendlyAsyncOverload": { "enabled": false },


let's rename it to csharpFriendlyAsyncOverload

knocte · 2021-12-20T04:03:59Z

src/FSharpLint.Core/Rules/Conventions/FriendlyAsyncOverload.fs

+                        { Range = range
+                          Message = String.Format(Resources.GetString "RulesFriendlyAsyncOverload", ident)
+                          SuggestedFix = None
+                          TypeChecks = [] }


use List.Empty here pls

knocte · 2021-12-20T04:04:55Z

src/FSharpLint.Core/Rules/Conventions/FriendlyAsyncOverload.fs

+                        Some(
+                            { Ident = fnIdent
+                              Range = range }
+                        )


let's use |> Some here to save 1 line of code and to not need parenthesis

knocte · 2021-12-20T04:05:32Z

src/FSharpLint.Core/Rules/Conventions/FriendlyAsyncOverload.fs

+            | head::_ when head.idText = "Async" ->
+                let idents = getIdentFromSynPat pattern
+                match idents with
+                | Some ident when ident.EndsWith("Async") = false ->


no need for parenthesis here (and let's use the operator not instead of = false)

knocte · 2021-12-20T15:33:26Z

src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs

+    | _ -> None
+
+let runner (args: AstNodeRuleParams) =
+    let hasAsync (syntaxArray: AbstractSyntaxArray.Node []) nodeIndex fnIdent =


use List<AbstractSyntaxArray.Node> here

I mean array<AbstractSyntaxArray.Node> not AbstractSyntaxArray.Node []

knocte · 2021-12-20T15:35:51Z

src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs

+                | AstNode.Binding (SynBinding (_, _, _, _, _attributes, _, _, pattern, _, _, range, _)) ->
+                    match getIdentFromSynPat pattern with
+                    | Some ident when ident = fnIdent + "Async" ->
+                            { Ident = fnIdent


4 space indentation, not 8

Fixes fsprojects#517

su8898 · 2021-12-20T15:50:57Z

@knocte I've addressed your review comments and updated the PR. Please have a look. thanks.

knocte · 2022-11-18T13:30:23Z

Superseded by #586

knocte reviewed Dec 20, 2021

View reviewed changes

su8898 force-pushed the CSharpFriendlyAsyncOverloads branch 3 times, most recently from ea40418 to e2a94e0 Compare December 20, 2021 15:31

knocte reviewed Dec 20, 2021

View reviewed changes

su8898 force-pushed the CSharpFriendlyAsyncOverloads branch from e2a94e0 to 590708e Compare December 20, 2021 15:33

knocte reviewed Dec 20, 2021

View reviewed changes

su8898 force-pushed the CSharpFriendlyAsyncOverloads branch from 590708e to afe40c9 Compare December 20, 2021 15:40

Add new rule C#-FriendlyAsyncOverload

ede5c57

Fixes fsprojects#517

su8898 force-pushed the CSharpFriendlyAsyncOverloads branch from afe40c9 to ede5c57 Compare December 20, 2021 15:43

knocte changed the title ~~Add new rule FriendlyAsyncOverload~~ Add new rule CSharpFriendlyAsyncOverload Mar 1, 2022

parhamsaremi mentioned this pull request Nov 18, 2022

DRAFT: Add new rule C#-FriendlyAsyncOverload #586

Draft

knocte closed this Nov 18, 2022


		## Rationale

		Exposing public F#-async APIs in a C#-friendly manner for better C# interoperability.


		## How To Fix

		Add an `Async` version of the API that returns a `Task<'T>`

Add new rule CSharpFriendlyAsyncOverload #533

Add new rule CSharpFriendlyAsyncOverload #533

Uh oh!

Conversation

su8898 commented Dec 20, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knocte Dec 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

su8898 commented Dec 20, 2021

Uh oh!

knocte commented Nov 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

knocte Dec 20, 2021 •

edited

Loading