-
Notifications
You must be signed in to change notification settings - Fork 73
Added SimpleAsyncComplementaryHelpers rule #841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added SimpleAsyncComplementaryHelpers rule #841
Conversation
Greptile OverviewConfidence Score: 4/5
|
| Filename | Overview |
|---|---|
| docs/content/how-tos/rules/FL0097.md | Documentation for new FL0097 rule; contains two typos ("asunchronous" and "asyncronous") |
| src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs | Main rule implementation; checks for missing complementary async/task helper functions |
| tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs | Comprehensive test suite with 11 test cases covering various scenarios |
| src/FSharpLint.Core/Application/Lint.fs | Adds Task-returning wrapper functions for existing Async functions (dogfooding the new rule) |
| src/FSharpLint.Core/Rules/Identifiers.fs | Adds identifier 97 for new rule; missing newline at end of file |
Sequence Diagram
sequenceDiagram
participant User
participant Linter as FSharpLint
participant Rule as SimpleAsyncComplementaryHelpers
participant AST as Syntax Tree
User->>Linter: Lint F# source code
Linter->>Rule: Process AST nodes
Rule->>AST: Extract module declarations
AST-->>Rule: Return bindings
loop For each binding
Rule->>Rule: Check if public function with args
alt Has Async prefix
Rule->>Rule: Extract base name (remove Async prefix)
Rule->>Rule: Mark as Async return type
else Has Async suffix
Rule->>Rule: Extract base name (remove Async suffix)
Rule->>Rule: Mark as Task return type
else No Async prefix/suffix
Rule->>Rule: Skip (not async function)
end
end
Rule->>Rule: Group functions by base name
alt Async function without Task complement
Rule->>Linter: Emit warning: "Create BarAsync() that calls Async.StartAsTask(AsyncBar())"
else Task function without Async complement
Rule->>Linter: Emit warning: "Create AsyncBar() that calls async { return Async.AwaitTask(BarAsync()) }"
else Both variants exist
Rule->>Linter: No warning (compliant)
end
Linter-->>User: Return lint results
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 files reviewed, 3 comments
66cc94a to
e6f3b12
Compare
|
After you have finished implementing v2:
Thanks |
113fb3f to
006c632
Compare
8d77fda to
c0bcef2
Compare
|
@knocte Github bugged out again and closed the PR when I was pushing 1-by-1. |
|
Reopened. Seems that it was some other bug in UI. |
|
Why is there a conflict? |
Greptile OverviewConfidence Score: 3/5
|
| Filename | Overview |
|---|---|
| docs/content/how-tos/rules/FL0097.md | Adds new rule documentation for FL0097, including config example and rationale. |
| src/FSharpLint.Core/Application/Configuration.fs | Registers SimpleAsyncComplementaryHelpers rule config field and adds it to enabled-rule construction. |
| src/FSharpLint.Core/Application/Lint.fs | Adds Task-returning wrapper APIs (lint*Async) around existing Async lint entrypoints using Async.StartAsTask. |
| src/FSharpLint.Core/Application/Lint.fsi | Extends public API surface with Task-returning lint*Async signatures matching new wrappers in Lint.fs. |
| src/FSharpLint.Core/FSharpLint.Core.fsproj | Adds new rule and utility source files to Core project and adjusts naming helper include path. |
| src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs | Implements new FL0097 rule to detect missing Async/Task twin helpers; contains performance issue in binding collection and file has UTF-8 BOM. |
| src/FSharpLint.Core/Rules/Identifiers.fs | Adds new identifier constant for SimpleAsyncComplementaryHelpers (97). |
| src/FSharpLint.Core/Rules/NamingHelper.fs | Pure file move/rename for NamingHelper to a shared Rules location (no content change). |
| src/FSharpLint.Core/Rules/Smells/NoAsyncRunSynchronouslyInLibrary.fs | Refactors library heuristics into shared Utilities.LibraryHeuristics module and updates imports. |
| src/FSharpLint.Core/Rules/Utilities.fs | Introduces Utilities.LibraryHeuristics module to share project-name heuristic across rules. |
| src/FSharpLint.Core/Text.resx | Adds two localized resource strings used by the new FL0097 rule messages. |
| src/FSharpLint.Core/fsharplint.json | Registers default config entry for simpleAsyncComplementaryHelpers (disabled by default, mode OnlyPublicAPIsInLibraries). |
| tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj | Adds new test file for SimpleAsyncComplementaryHelpers rule to test project. |
| tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs | Adds unit tests covering violations/non-violations and mode behavior for the new rule. |
| tests/FSharpLint.Core.Tests/Rules/Smells/NoAsyncRunSynchronouslyInLibrary.fs | Updates tests to use shared Utilities.LibraryHeuristics import after refactor. |
Sequence Diagram
sequenceDiagram
actor Caller
participant Lint as FSharpLint.Application.Lint
participant AsyncCore as asyncLint* (Async)
participant TaskWrap as lint*Async (Task wrapper)
participant Config as Configuration.parseConfig
participant Rules as AstNodeRuleRunner
Caller->>Lint: lintProjectAsync / lintSolutionAsync / lintFileAsync / lintSourceAsync
Lint->>TaskWrap: Async.StartAsTask(asyncLint*)
TaskWrap->>AsyncCore: run asyncLint* workflow
AsyncCore->>Config: load/parse configuration (if needed)
AsyncCore->>Rules: run rules incl. SimpleAsyncComplementaryHelpers
Rules-->>AsyncCore: violations
AsyncCore-->>TaskWrap: LintResult
TaskWrap-->>Caller: Task<LintResult>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
15 files reviewed, 2 comments
src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs
Show resolved
Hide resolved
src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs
Show resolved
Hide resolved
And a test for it.
For false-positive on non-function values.
For async functions that start with lowercase character.
That has Mode member, which may be on of: OnlyPublicAPIsInLibraries (default), AnyPublicAPIs, AllAPIs.
From NoAsyncRunSynchronouslyInLibrary to new module Rules.Utilities.LibraryHeuristics, becuase it will also be used by SimpleAsyncComplementaryHelpers rule. Also had to move NamingHelper.fs to parent dir (Rules) because it's used by LibraryHeuristics and must appear before Utilities.fs file.
For AllAPIs mode.
Mode OnlyPublicAPIsInLibraries only runs in projects that are likely library projects. Mode AnyPublicAPIs flags all public APIs like before. Mode AllAPIs also flags (give violations about) internal, private, and nested APIs.
4d9dbf0 to
973bf2c
Compare
Because I moved library heuristics from NoAsyncRunSynchronouslyInLibrary to new module. Rebased, no conflict now. |
Contributes to #517.