Skip to content

Commit 5fd36ad

Browse files
Smaug123Smaug123
committed
Markups
1 parent a311901 commit 5fd36ad

4 files changed

Lines changed: 43 additions & 55 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
## 0.19.0 - 2025-08-27
44

55
### Added
6-
* New analyzer, SyncBlockingAnalyzer, which bans the use of synchronous blocking methods like `Task.Wait`, unless specifically permitted per-line with a magic comment.
6+
* New analyzer, SyncBlockingAnalyzer, which bans the use of synchronous blocking methods like `Task.Wait`, unless specifically permitted per-line with a magic comment. [#96](https://github.com/G-Research/fsharp-analyzers/pull/96)
77

88
## 0.18.0 - 2025-08-27
99

‎src/FSharp.Analyzers/Comments.fs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ let isSwitchedOffPerComment
1717
comments
1818
|> List.exists (fun c ->
1919
match c with
20+
| CommentTrivia.BlockComment r
2021
| CommentTrivia.LineComment r ->
2122
if r.StartLine <> analyzerTriggeredOn.StartLine - 1 then
2223
false
2324
else
2425
let lineOfComment = sourceText.GetLineString (r.StartLine - 1) // 0-based
2526

2627
lineOfComment.Contains (magicComment, StringComparison.OrdinalIgnoreCase)
27-
| _ -> false
2828
)

‎src/FSharp.Analyzers/SyncBlockingAnalyzer.fs‎

Lines changed: 35 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
module GR.FSharp.Analyzers.SyncBlockingAnalyzer
22

33
open System
4+
open FSharp.Analyzers.Comments
45
open FSharp.Analyzers.SDK
56
open FSharp.Analyzers.SDK.TASTCollecting
67
open FSharp.Compiler.CodeAnalysis
@@ -17,7 +18,7 @@ let SwitchOffComment = "synchronous blocking call allowed"
1718

1819
let problematicMethods =
1920
[
20-
"Microsoft.FSharp.Control.RunSynchronously" // This is how it appears in the TAST
21+
"Microsoft.FSharp.Control.RunSynchronously" // This is how it appears in the TAST
2122
"Microsoft.FSharp.Control.Async.RunSynchronously"
2223
"Microsoft.FSharp.Control.FSharpAsync.RunSynchronously"
2324
"System.Threading.Tasks.Task.Wait"
@@ -32,67 +33,50 @@ let problematicMethods =
3233

3334
let problematicProperties =
3435
[
35-
"System.Threading.Tasks.Task.get_Result" // Note: F# doesn't include `1 in FullName for generic type property getters
36-
"System.Threading.Tasks.ValueTask.get_Result" // Same here
36+
"System.Threading.Tasks.Task.get_Result" // Note: F# doesn't include `1 in FullName for generic type property getters
37+
"System.Threading.Tasks.ValueTask.get_Result" // Same here
3738
]
3839
|> Set.ofList
3940

40-
let isSwitchedOffPerComment (sourceText: ISourceText) (comments: CommentTrivia list) (range: Range) =
41-
comments
42-
|> List.exists (fun c ->
43-
match c with
44-
| CommentTrivia.LineComment r ->
45-
// Check same line or line above
46-
if r.StartLine = range.StartLine || r.StartLine = range.StartLine - 1 then
47-
let lineOfComment = sourceText.GetLineString(r.StartLine - 1) // 0-based
48-
lineOfComment.Contains(SwitchOffComment, StringComparison.OrdinalIgnoreCase)
49-
else
50-
false
51-
| CommentTrivia.BlockComment r ->
52-
// Check if block comment is on same line or line above
53-
if r.StartLine = range.StartLine || r.EndLine = range.StartLine - 1 then
54-
let startLine = sourceText.GetLineString(r.StartLine - 1)
55-
startLine.Contains(SwitchOffComment, StringComparison.OrdinalIgnoreCase)
56-
else
57-
false
58-
)
59-
60-
let analyze (sourceText: ISourceText) (ast: ParsedInput) (checkFileResults: FSharpCheckFileResults) =
41+
let analyze (sourceText : ISourceText) (ast : ParsedInput) (checkFileResults : FSharpCheckFileResults) =
6142
let comments =
6243
match ast with
6344
| ParsedInput.ImplFile parsedImplFileInput -> parsedImplFileInput.Trivia.CodeComments
6445
| _ -> []
6546

66-
let violations = ResizeArray<range * string * string>()
47+
let violations = ResizeArray<range * string * string> ()
6748

6849
let walker =
6950
{ new TypedTreeCollectorBase() with
70-
override _.WalkCall _ (mfv: FSharpMemberOrFunctionOrValue) _ _ _ (m: range) =
71-
51+
override _.WalkCall _ (mfv : FSharpMemberOrFunctionOrValue) _ _ _ (m : range) =
52+
7253
// Check for regular method calls
7354
if problematicMethods.Contains mfv.FullName then
74-
if not (isSwitchedOffPerComment sourceText comments m) then
55+
if not (isSwitchedOffPerComment SwitchOffComment comments sourceText m) then
7556
let methodName =
76-
if mfv.DisplayName.Contains(".") then
57+
if mfv.DisplayName.Contains '.' then
7758
mfv.DisplayName
78-
else
59+
else if
7960
// Special handling for Async.RunSynchronously
80-
if mfv.FullName = "Microsoft.FSharp.Control.RunSynchronously" then
81-
"Async.RunSynchronously"
61+
mfv.FullName = "Microsoft.FSharp.Control.RunSynchronously"
62+
then
63+
"Async.RunSynchronously"
64+
else
65+
// Get more context for better error messages
66+
let parts = mfv.FullName.Split '.'
67+
68+
if parts.Length >= 2 then
69+
$"{parts.[parts.Length - 2]}.{parts.[parts.Length - 1]}"
8270
else
83-
// Get more context for better error messages
84-
let parts = mfv.FullName.Split('.')
85-
if parts.Length >= 2 then
86-
$"{parts.[parts.Length - 2]}.{parts.[parts.Length - 1]}"
87-
else
88-
mfv.DisplayName
89-
violations.Add(m, "method", methodName)
90-
71+
mfv.DisplayName
72+
73+
violations.Add (m, "method", methodName)
74+
9175
// Check for property getters (Result property access)
9276
elif problematicProperties.Contains mfv.FullName then
93-
if not (isSwitchedOffPerComment sourceText comments m) then
77+
if not (isSwitchedOffPerComment SwitchOffComment comments sourceText m) then
9478
// For property getters, use just the property name
95-
violations.Add(m, "property", "Result")
79+
violations.Add (m, "property", "Result")
9680
}
9781

9882
match checkFileResults.ImplementationFile with
@@ -104,10 +88,10 @@ let analyze (sourceText: ISourceText) (ast: ParsedInput) (checkFileResults: FSha
10488
{
10589
Type = "SyncBlockingAnalyzer"
10690
Message =
107-
$"Synchronous blocking call '%s{name}' should be avoided. " +
108-
"This can cause deadlocks and thread pool starvation. " +
109-
"Consider using `let!` in a `task` or `async` computation expression. " +
110-
"Suppress with comment including text 'synchronous blocking call allowed'."
91+
$"Synchronous blocking call '%s{name}' should be avoided. "
92+
+ "This can cause deadlocks and thread pool starvation. "
93+
+ "Consider using `let!` in a `task` or `async` computation expression. "
94+
+ "Suppress with comment including text 'synchronous blocking call allowed'."
11195
Code = Code
11296
Severity = Severity.Warning
11397
Range = range
@@ -128,16 +112,16 @@ let HelpUri =
128112
"https://g-research.github.io/fsharp-analyzers/analyzers/SyncBlockingAnalyzer.html"
129113

130114
[<CliAnalyzer(Name, ShortDescription, HelpUri)>]
131-
let syncBlockingCliAnalyzer: Analyzer<CliContext> =
132-
fun (ctx: CliContext) ->
115+
let syncBlockingCliAnalyzer : Analyzer<CliContext> =
116+
fun (ctx : CliContext) ->
133117
async { return analyze ctx.SourceText ctx.ParseFileResults.ParseTree ctx.CheckFileResults }
134118

135119
[<EditorAnalyzer(Name, ShortDescription, HelpUri)>]
136-
let syncBlockingEditorAnalyzer: Analyzer<EditorContext> =
137-
fun (ctx: EditorContext) ->
120+
let syncBlockingEditorAnalyzer : Analyzer<EditorContext> =
121+
fun (ctx : EditorContext) ->
138122
async {
139123
return
140124
ctx.CheckFileResults
141125
|> Option.map (analyze ctx.SourceText ctx.ParseFileResults.ParseTree)
142126
|> Option.defaultValue []
143-
}
127+
}

‎tests/FSharp.Analyzers.Tests/data/syncBlocking/negative/With suppression comment.fs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ let testAsyncRunSynchronouslyWithComment () =
1010

1111
let testTaskWaitWithComment () =
1212
let t = Task.Run(fun () -> 42)
13-
t.Wait() // synchronous blocking call allowed for testing
13+
t
14+
// ANALYZER: synchronous blocking call allowed for testing
15+
|> fun x -> x.Wait()
16+
// ANALYZER: synchronous blocking call allowed for testing
1417
t.Result
1518

1619
let testTaskResultWithCommentAbove () =
@@ -26,4 +29,5 @@ let testWithBlockComment () =
2629
let testGetResultWithComment () =
2730
let t = Task.Run(fun () -> 42)
2831
let awaiter = t.GetAwaiter()
29-
awaiter.GetResult() // synchronous blocking call allowed in main
32+
// synchronous blocking call allowed in main
33+
awaiter.GetResult()

0 commit comments

Comments
 (0)