Skip to content

Conversation

@webwarrior-ws
Copy link
Contributor

Contributes to #517.

@greptile-apps
Copy link

greptile-apps bot commented Feb 9, 2026

Greptile Overview

Confidence Score: 4/5

  • Safe to merge with minor documentation fixes recommended
  • The implementation is solid with comprehensive tests and proper integration. The only issues are cosmetic: two typos in documentation and a missing newline. The rule logic is well-designed and the PR demonstrates good practices by applying the new rule to the codebase itself.
  • docs/content/how-tos/rules/FL0097.md requires spelling corrections

Important Files Changed

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
Loading

Copy link

@greptile-apps greptile-apps bot left a 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

Edit Code Review Agent Settings | Greptile

@webwarrior-ws webwarrior-ws force-pushed the simple-async-complementary-helpers-2-squashed branch from 66cc94a to e6f3b12 Compare February 10, 2026 08:37
@knocte
Copy link
Collaborator

knocte commented Feb 10, 2026

After you have finished implementing v2:

  • Change violation msgs from "Create foo bar that just calls Async.StartAsTask(baz bazz)." to "Consider creating twin helper function/method foo bar that just calls Async.StartAsTask(baz bazz) under the hood".
  • Squash commits aeb331f, e66755c and 356fe55 into one.
  • Click on the "Resolve conversation buttons" of threads that have been addressed (yes, including the comments that greptile-apps bot raised).

Thanks

@webwarrior-ws webwarrior-ws force-pushed the simple-async-complementary-helpers-2-squashed branch from 113fb3f to 006c632 Compare February 10, 2026 09:38
@webwarrior-ws webwarrior-ws force-pushed the simple-async-complementary-helpers-2-squashed branch from 8d77fda to c0bcef2 Compare February 10, 2026 10:41
@webwarrior-ws
Copy link
Contributor Author

@knocte Github bugged out again and closed the PR when I was pushing 1-by-1.
I can't reopen it myself.

@webwarrior-ws webwarrior-ws reopened this Feb 10, 2026
@webwarrior-ws
Copy link
Contributor Author

Reopened. Seems that it was some other bug in UI.

@knocte
Copy link
Collaborator

knocte commented Feb 10, 2026

Why is there a conflict?

@greptile-apps
Copy link

greptile-apps bot commented Feb 10, 2026

Greptile Overview

Confidence Score: 3/5

  • This PR is mergeable after fixing the rule implementation issues noted in SimpleAsyncComplementaryHelpers.
  • Most changes are straightforward wiring/tests/docs, but the new rule implementation introduces a guaranteed O(n²) traversal pattern in binding collection and the source file includes a UTF-8 BOM that can cause tooling/compiler issues in some environments. Tests/build could not be executed in this environment due to missing dotnet.
  • src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs

Important Files Changed

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>
Loading

Copy link

@greptile-apps greptile-apps bot left a 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

Edit Code Review Agent Settings | Greptile

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.
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.
@webwarrior-ws webwarrior-ws force-pushed the simple-async-complementary-helpers-2-squashed branch from 4d9dbf0 to 973bf2c Compare February 10, 2026 10:57
@webwarrior-ws
Copy link
Contributor Author

Why is there a conflict?

Because I moved library heuristics from NoAsyncRunSynchronouslyInLibrary to new module.

Rebased, no conflict now.

@knocte knocte merged commit d04a873 into fsprojects:master Feb 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants