From bf0bbe7495c359cb08bbd672ff8e40ee760e4ea1 Mon Sep 17 00:00:00 2001 From: Smaug123 <3138005+Smaug123@users.noreply.github.com> Date: Wed, 27 Aug 2025 11:57:58 +0100 Subject: [PATCH 1/3] Extract comment-searching code --- src/FSharp.Analyzers/Comments.fs | 28 ++++++++++++++++++ .../DisposedBeforeAsyncRunAnalyzer.fs | 29 ++++++------------- src/FSharp.Analyzers/FSharp.Analyzers.fsproj | 3 +- .../negative/CommentOnUseBeforeAsync.fs | 2 +- .../negative/CommentOnUseBeforeTask.fs | 2 +- 5 files changed, 41 insertions(+), 23 deletions(-) create mode 100644 src/FSharp.Analyzers/Comments.fs diff --git a/src/FSharp.Analyzers/Comments.fs b/src/FSharp.Analyzers/Comments.fs new file mode 100644 index 0000000..4e76f9e --- /dev/null +++ b/src/FSharp.Analyzers/Comments.fs @@ -0,0 +1,28 @@ +module FSharp.Analyzers.Comments + +open System +open FSharp.Compiler.SyntaxTrivia +open FSharp.Compiler.Text + +/// A standard pattern within an analyzer is to look for a specific comment preceding a problematic line, +/// indicating "suppress the analyzer on this line". +/// This function performs that common check. +let isSwitchedOffPerComment + (magicComment : string) + (comments : CommentTrivia list) + (sourceText : ISourceText) + (analyzerTriggeredOn : Range) + : bool + = + comments + |> List.exists (fun c -> + match c with + | CommentTrivia.LineComment r -> + if r.StartLine <> analyzerTriggeredOn.StartLine - 1 then + false + else + let lineOfComment = sourceText.GetLineString (r.StartLine - 1) // 0-based + + lineOfComment.Contains (magicComment, StringComparison.OrdinalIgnoreCase) + | _ -> false + ) diff --git a/src/FSharp.Analyzers/DisposedBeforeAsyncRunAnalyzer.fs b/src/FSharp.Analyzers/DisposedBeforeAsyncRunAnalyzer.fs index dbc892c..91bad07 100644 --- a/src/FSharp.Analyzers/DisposedBeforeAsyncRunAnalyzer.fs +++ b/src/FSharp.Analyzers/DisposedBeforeAsyncRunAnalyzer.fs @@ -1,6 +1,7 @@ module GR.FSharp.Analyzers.DisposedBeforeAsyncRunAnalyzer open System +open FSharp.Analyzers.Comments open FSharp.Analyzers.SDK open FSharp.Analyzers.SDK.ASTCollecting open FSharp.Compiler.CodeAnalysis @@ -65,10 +66,7 @@ let pathContainsComputationExpr (path : SyntaxVisitorPath) = ) [] -let SwitchOffAsyncComment = "disposed before returned async runs" - -[] -let SwitchOffTaskComment = "disposed before returned task runs" +let SwitchOffComment = "disposed before returned workflow runs" let collectUses (sourceText : ISourceText) (ast : ParsedInput) (checkFileResults : FSharpCheckFileResults) = let comments = @@ -76,21 +74,6 @@ let collectUses (sourceText : ISourceText) (ast : ParsedInput) (checkFileResults | ParsedInput.ImplFile parsedImplFileInput -> parsedImplFileInput.Trivia.CodeComments | _ -> [] - let isSwitchedOffPerComment (range : Range) = - comments - |> List.exists (fun c -> - match c with - | CommentTrivia.LineComment r -> - if r.StartLine <> range.StartLine - 1 then - false - else - let lineOfComment = sourceText.GetLineString (r.StartLine - 1) // 0-based - - lineOfComment.Contains (SwitchOffAsyncComment, StringComparison.OrdinalIgnoreCase) - || lineOfComment.Contains (SwitchOffTaskComment, StringComparison.OrdinalIgnoreCase) - | _ -> false - ) - let uses = ResizeArray () // Note: not tailrecursive @@ -117,7 +100,13 @@ let collectUses (sourceText : ISourceText) (ast : ParsedInput) (checkFileResults match synExpr with | SynExpr.LetOrUse (isUse = true ; bindings = [ binding ] ; body = body) -> if - not (isSwitchedOffPerComment binding.RangeOfBindingWithoutRhs) + not ( + isSwitchedOffPerComment + SwitchOffComment + comments + sourceText + binding.RangeOfBindingWithoutRhs + ) && hasAsyncOrTaskInBody body then match pathContainsAsyncOrTaskReturningFunc checkFileResults sourceText path with diff --git a/src/FSharp.Analyzers/FSharp.Analyzers.fsproj b/src/FSharp.Analyzers/FSharp.Analyzers.fsproj index 6f22233..d648b31 100644 --- a/src/FSharp.Analyzers/FSharp.Analyzers.fsproj +++ b/src/FSharp.Analyzers/FSharp.Analyzers.fsproj @@ -15,6 +15,7 @@ + @@ -43,4 +44,4 @@ - \ No newline at end of file + diff --git a/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeAsync.fs b/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeAsync.fs index aea4d7b..5f067fb 100644 --- a/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeAsync.fs +++ b/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeAsync.fs @@ -7,7 +7,7 @@ type DisposableThing () = member _.Dispose() = () let asyncReturningFunc () = - // Note: disposed before returned async runs + // Note: disposed before returned workflow runs use t = new DisposableThing() async { return "hi" diff --git a/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeTask.fs b/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeTask.fs index af34b74..87700fe 100644 --- a/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeTask.fs +++ b/tests/FSharp.Analyzers.Tests/data/disposedBeforeAsyncRun/negative/CommentOnUseBeforeTask.fs @@ -7,7 +7,7 @@ type DisposableThing () = member _.Dispose() = () let taskReturningFunc () = - // Note: disposed before returned task runs + // Note: disposed before returned workflow runs use t = new DisposableThing() task { return "hi" From 98ec4061789661c37cce0333433d167ceaee8ce1 Mon Sep 17 00:00:00 2001 From: Smaug123 <3138005+Smaug123@users.noreply.github.com> Date: Wed, 27 Aug 2025 12:08:28 +0100 Subject: [PATCH 2/3] Changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e262f0..736b246 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 0.18.0 - 2025-08-27 + +### Changed +* The magic string for the DisposedBeforeAsyncRunAnalyzer has changed: it is now "disposed before returned workflow runs". [#94](https://github.com/G-Research/fsharp-analyzers/pull/94) + ## 0.17.0 - 2025-07-12 ### Changed From a4997d91c5326428b19209d90a0cd24aefd2c11d Mon Sep 17 00:00:00 2001 From: Smaug123 <3138005+Smaug123@users.noreply.github.com> Date: Wed, 27 Aug 2025 12:15:15 +0100 Subject: [PATCH 3/3] Docs --- docs/analyzers/DisposedBeforeAsyncRunAnalyzer.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/analyzers/DisposedBeforeAsyncRunAnalyzer.md b/docs/analyzers/DisposedBeforeAsyncRunAnalyzer.md index 82d01d8..37e29d4 100644 --- a/docs/analyzers/DisposedBeforeAsyncRunAnalyzer.md +++ b/docs/analyzers/DisposedBeforeAsyncRunAnalyzer.md @@ -31,10 +31,10 @@ let f () = } ``` -If the `use` before the `async` is really your intent, you can disable the warning with a line comment on top of the `use` statement containing `disposed before returned async runs` or `disposed before returned task runs`. +If the `use` before the `async` is really your intent, you can disable the warning with a line comment on top of the `use` statement containing `disposed before returned workflow runs`. ```fsharp let f () = - // Note: disposed before returned async runs + // Note: disposed before returned workflow runs use t = new DisposableThing() async { return "hi"