Add NoAsyncRunSynchronouslyInLibrary rule #785

webwarrior-ws · 2025-11-17T13:16:34Z

Added new NoAsyncRunSynchronouslyInLibrary rule and tests for it.

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

src/FSharpLint.Core/Text.resx

tests/FSharpLint.Core.Tests/Rules/Conventions/NoAsyncRunSynchronouslyInLibrary.fs

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

knocte · 2025-11-18T11:42:06Z

Make rule pass new tests by checking all nodes in the file for test attributes

But we don't want the check to be just about the file but about the whole assembly.

webwarrior-ws · 2025-11-18T11:44:02Z

Make rule pass new tests by checking all nodes in the file for test attributes

But we don't want the check to be just about the file but about the whole assembly.

The whole assembly check will only work when linting project or solution.

knocte · 2025-11-18T11:46:33Z

The whole assembly check will only work when linting project or solution.

I know. That's why we don't recommend (show a warning) when only linting files.

knocte · 2025-11-18T11:55:44Z

BTW, in my comment (#785 (comment)) I'm quoting some text from a commit msg, that either should be fixed (changed the wording), or implemented properly.

knocte · 2025-11-18T12:27:43Z

BTW, in my comment (#785 (comment)) I'm quoting some text from a commit msg, that either should be fixed (changed the wording), or implemented properly.

After you have fixed the above, let's add 2 more commits:

1st will be, in all cases where you have changed the API of FSharpLint.Core in this PR (for example to change Foo(): Bar to AsyncFoo(): Async<Bar>), reintroduce the old method/function: Foo() that will simply call Async.RunSyncronously AsyncFoo(). CI of this commit will obviously fail on SelfCheck.
2nd commit will decorate all these methods with an Obsolete attrib, will also change the rule to ignore methods/functions with this attrib, add 2 more tests, and its CI should be green.

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

src/FSharpLint.Console/Program.fs

src/FSharpLint.Core/Application/Lint.fs

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

No checks for assembly name yet.

Check assembly name. If it contains "test" substring, assume this is test project and don't fire the rule.

For NoAsyncRunSynchronouslyInLibrary rule that make sure that code inside NUnit and MSTest tests doesn't trigger the rule. Includes failing tests.

Check if Async.RunSynchronously call is iniside NUnit or MSTest tests. If it is, don't trigger the rule.

Add configuration for the rule and enable it by default.

Assembly.QualifiedName is empty, so instead check namespace and project name.

Simplified check for entry point. Also combined some code that uses args.CheckInfo to not pattern match on it twice.

That also calls `Async.RunSynchronously` and causes CI to fail.

Got rid of Async.RunSynchronously calls in FSharpLint.Core.

knocte · 2026-01-06T10:38:44Z

let's squash commit Revert "Benchmarks: suppress rule" because:

its commit msg got obsolete, after the rebases
there shouldn't be reverts of commits that live in the same PR

Exclude projects that have "console" in their name addition to those that contain "test".

Updated docs for NoAsyncRunSynchronouslyInLibrary to make clear what code is considered to be library code.

When linting project. This will give more information for rules to use, such as all types defined in an assembly.

For EntryPoint point attribute instead of just current file.

Added 2 more tests for NoAsyncRunSynchronouslyInLibrary rule.

Make rule pass new tests by checking all nodes in the file for test attributes, not only parent nodes. Also when checking a project, check all files in the project for classes that are marked by test attributes.

That were removed in commit 9de63ea ("Core,Console,Tests: refactoring").

Methods and functions when applying the rule. Added 2 more tests. Marked non-async parsing methods re-introduced in previous commit with `[<Obsolete>]` attribute.

To address feedback.

Use new function `howLikelyProjectIsTestOrLibrary` instead of `isInTestProject` and made unit tests for this function.

Category, because its violations are more like a mistake, not a deviation from a certain standard. Also moved Smells folder down in the project, because NoAsyncRunSynchronouslyInLibrary depends on function from NamingHelpers, and that module is in Conventions/Naming. Conventions folder was above Smells, which made NamingHelpers inaccessible to NoAsyncRunSynchronouslyInLibrary.

src/FSharpLint.Core/Rules/Smells/NoAsyncRunSynchronouslyInLibrary.fs

In addition to class attribute when checking file declarations to decide if the class is test class because in later NUnit versions it is optional to mark the class, only methods must be marked with test attribute.

knocte reviewed Nov 17, 2025

View reviewed changes

docs/content/how-tos/rules/FL0087.md Outdated Show resolved Hide resolved

knocte reviewed Nov 17, 2025

View reviewed changes

docs/content/how-tos/rules/FL0087.md Outdated Show resolved Hide resolved

knocte reviewed Nov 17, 2025

View reviewed changes

src/FSharpLint.Core/Text.resx Outdated Show resolved Hide resolved

knocte reviewed Nov 17, 2025

View reviewed changes

tests/FSharpLint.Core.Tests/Rules/Conventions/NoAsyncRunSynchronouslyInLibrary.fs Show resolved Hide resolved

knocte reviewed Nov 17, 2025

View reviewed changes

src/FSharpLint.Core/Rules/Conventions/NoAsyncRunSynchronouslyInLibrary.fs Outdated Show resolved Hide resolved

webwarrior-ws force-pushed the no-run-synchronously branch 4 times, most recently from 8fbe79e to 43fa62a Compare November 18, 2025 11:39

webwarrior-ws force-pushed the no-run-synchronously branch 3 times, most recently from 75e8b61 to b83aebd Compare November 19, 2025 08:35