From 5d8096175e532ad20bf055ed3ea667090965e61f Mon Sep 17 00:00:00 2001 From: Michael Glass Date: Fri, 3 Jul 2026 13:22:58 +0200 Subject: [PATCH] feat(Core): supply project file name to library-heuristic rules under TransparentCompiler The library-heuristic rules (AsynchronousFunctionNames, SimpleAsyncComplementaryHelpers, NoAsyncRunSynchronouslyInLibrary) need only the project file name. The two-phase ProjectOptions lazy derived it via FSharpProjectContext.ProjectOptions, which throws by design when the check results come from FCS's TransparentCompiler, aborting linting of the whole file. Expose the project file name to rules directly (AstNodeRuleParams.ProjectFileName) and let callers supply it (ParsedFileInformation.ProjectFileName / FileParseInfo.ProjectFileName). Project-mode lint fills it from the FSharpProjectOptions it already builds; analyzer hosts can supply their .fsproj path. Deriving it from the check results remains as a guarded fallback, so behavior is preserved everywhere and no longer depends on a getter the TransparentCompiler cannot serve. Regression: TestApi lints TransparentCompiler-produced results without an internal failure, and verifies a caller-supplied library project name engages the async-naming rule under the TransparentCompiler. --- src/FSharpLint.Core/Application/Lint.fs | 36 ++++++-- src/FSharpLint.Core/Application/Lint.fsi | 6 +- src/FSharpLint.Core/Framework/ParseFile.fs | 6 ++ src/FSharpLint.Core/Framework/Rules.fs | 2 +- .../Naming/AsynchronousFunctionNames.fs | 4 +- .../Naming/SimpleAsyncComplementaryHelpers.fs | 4 +- .../NoAsyncRunSynchronouslyInLibrary.fs | 6 +- tests/FSharpLint.Benchmarks/Benchmark.fs | 2 +- .../Rules/TestAstNodeRule.fs | 2 +- .../Rules/TestHintMatcherBase.fs | 2 +- .../Rules/TestIndentationRule.fs | 2 +- .../Rules/TestLineRule.fs | 2 +- .../Rules/TestNoTabCharactersRule.fs | 2 +- tests/FSharpLint.FunctionalTest/TestApi.fs | 82 ++++++++++++++++++- 14 files changed, 133 insertions(+), 25 deletions(-) diff --git a/src/FSharpLint.Core/Application/Lint.fs b/src/FSharpLint.Core/Application/Lint.fs index ac91adea9..c0026152e 100644 --- a/src/FSharpLint.Core/Application/Lint.fs +++ b/src/FSharpLint.Core/Application/Lint.fs @@ -125,7 +125,7 @@ module Lint = GlobalConfig: Rules.GlobalRuleConfig TypeCheckResults: FSharpCheckFileResults option ProjectCheckResults: FSharpCheckProjectResults option - ProjectOptions: Lazy + ProjectFileName: Lazy FilePath: string FileContent: string Lines: string[] @@ -150,7 +150,7 @@ module Lint = Lines = config.Lines CheckInfo = config.TypeCheckResults ProjectCheckInfo = config.ProjectCheckResults - ProjectOptions = config.ProjectOptions + ProjectFileName = config.ProjectFileName GlobalConfig = config.GlobalConfig } // Build state for rules with context. @@ -265,9 +265,18 @@ module Lint = GlobalConfig = enabledRules.GlobalConfig TypeCheckResults = fileInfo.TypeCheckResults ProjectCheckResults = fileInfo.ProjectCheckResults - ProjectOptions = lazy( - fileInfo.ProjectCheckResults - |> Option.map _.ProjectContext.ProjectOptions + ProjectFileName = lazy( + fileInfo.ProjectFileName + |> Option.orElseWith (fun () -> + // A caller (project-mode lint, or an analyzer host) that knows the + // project file supplies it directly above. Otherwise derive it from + // the check results — but FSharpProjectContext.ProjectOptions throws + // by design under FCS's TransparentCompiler, so guard the access. + try + fileInfo.ProjectCheckResults + |> Option.map (fun projectCheckResults -> + projectCheckResults.ProjectContext.ProjectOptions.ProjectFileName) + with _ -> None) ) FilePath = fileInfo.File FileContent = fileInfo.Text @@ -399,6 +408,10 @@ module Lint = TypeCheckResults:FSharpCheckFileResults option /// Optional results of project-wide type info (allows for a more accurate lint). ProjectCheckResults:FSharpCheckProjectResults option + /// Path to the project file (.fsproj), when known. Lets the library-heuristic + /// rules work under hosts (e.g. TransparentCompiler analyzer hosts) where the + /// project options cannot be derived from the check results. + ProjectFileName:string option } /// Gets a FSharpLint Configuration based on the provided ConfigurationParam. @@ -468,7 +481,7 @@ module Lint = |> Array.iter (fun fileParseResult -> lint lintInformation - { fileParseResult with ProjectCheckResults = Some projectCheckResults }) + { fileParseResult with ProjectCheckResults = Some projectCheckResults; ProjectFileName = Some projectOptions.ProjectFileName }) return Success () else @@ -581,6 +594,7 @@ module Lint = ParseFile.Ast = parsedFileInfo.Ast ParseFile.TypeCheckResults = parsedFileInfo.TypeCheckResults ParseFile.ProjectCheckResults = parsedFileInfo.ProjectCheckResults + ParseFile.ProjectFileName = parsedFileInfo.ProjectFileName ParseFile.File = "" } lint lintInformation parsedFileInfo @@ -600,7 +614,8 @@ module Lint = { Source = parseFileInformation.Text Ast = parseFileInformation.Ast TypeCheckResults = parseFileInformation.TypeCheckResults - ProjectCheckResults = None } + ProjectCheckResults = None + ProjectFileName = None } return lintParsedSource optionalParams parsedFileInfo | ParseFile.Failed failure -> return LintResult.Failure(FailedToParseFile failure) @@ -635,6 +650,7 @@ module Lint = ParseFile.Ast = parsedFileInfo.Ast ParseFile.TypeCheckResults = parsedFileInfo.TypeCheckResults ParseFile.ProjectCheckResults = parsedFileInfo.ProjectCheckResults + ParseFile.ProjectFileName = parsedFileInfo.ProjectFileName ParseFile.File = filePath } lint lintInformation parsedFileInfo @@ -653,7 +669,8 @@ module Lint = { Source = astFileParseInfo.Text Ast = astFileParseInfo.Ast TypeCheckResults = astFileParseInfo.TypeCheckResults - ProjectCheckResults = astFileParseInfo.ProjectCheckResults } + ProjectCheckResults = astFileParseInfo.ProjectCheckResults + ProjectFileName = astFileParseInfo.ProjectFileName } return lintParsedFile optionalParams parsedFileInfo filePath | ParseFile.Failed failure -> return LintResult.Failure(FailedToParseFile failure) @@ -684,7 +701,8 @@ module Lint = { Source = astFileParseInfo.Text Ast = astFileParseInfo.Ast TypeCheckResults = astFileParseInfo.TypeCheckResults - ProjectCheckResults = astFileParseInfo.ProjectCheckResults } + ProjectCheckResults = astFileParseInfo.ProjectCheckResults + ProjectFileName = astFileParseInfo.ProjectFileName } return lintParsedFile optionalParams parsedFileInfo filePath | ParseFile.Failed failure -> return LintResult.Failure (FailedToParseFile failure) diff --git a/src/FSharpLint.Core/Application/Lint.fsi b/src/FSharpLint.Core/Application/Lint.fsi index 7128eadb7..0324a90d3 100644 --- a/src/FSharpLint.Core/Application/Lint.fsi +++ b/src/FSharpLint.Core/Application/Lint.fsi @@ -78,6 +78,10 @@ module Lint = /// Optional results of project-wide type info (allows for a more accurate lint). ProjectCheckResults:FSharpCheckProjectResults option + /// Path to the project file (.fsproj), when known. Lets the library-heuristic + /// rules work under hosts (e.g. TransparentCompiler analyzer hosts) where the + /// project options cannot be derived from the check results. + ProjectFileName:string option } type BuildFailure = | InvalidProjectFileMessage of string @@ -129,7 +133,7 @@ module Lint = GlobalConfig: Rules.GlobalRuleConfig TypeCheckResults: FSharpCheckFileResults option ProjectCheckResults: FSharpCheckProjectResults option - ProjectOptions: Lazy + ProjectFileName: Lazy FilePath: string FileContent: string Lines: string[] diff --git a/src/FSharpLint.Core/Framework/ParseFile.fs b/src/FSharpLint.Core/Framework/ParseFile.fs index 1bad7c1ca..c07506b0b 100644 --- a/src/FSharpLint.Core/Framework/ParseFile.fs +++ b/src/FSharpLint.Core/Framework/ParseFile.fs @@ -28,6 +28,11 @@ module ParseFile = /// Path to the file. File:string + + /// Path to the project file (.fsproj) this file belongs to, when known. + /// Callers can supply this so the library-heuristic rules work even when the + /// project options cannot be derived from the check results. + ProjectFileName:string option } [] @@ -54,6 +59,7 @@ module ParseFile = TypeCheckResults = Some(typeCheckResults) ProjectCheckResults = None File = file + ProjectFileName = None } | FSharpCheckFileAnswer.Aborted -> return Failed(AbortedTypeCheck) } diff --git a/src/FSharpLint.Core/Framework/Rules.fs b/src/FSharpLint.Core/Framework/Rules.fs index 38c8b2355..d4113c462 100644 --- a/src/FSharpLint.Core/Framework/Rules.fs +++ b/src/FSharpLint.Core/Framework/Rules.fs @@ -31,7 +31,7 @@ type AstNodeRuleParams = Lines:string [] CheckInfo:FSharpCheckFileResults option ProjectCheckInfo:FSharpCheckProjectResults option - ProjectOptions: Lazy + ProjectFileName: Lazy GlobalConfig:GlobalRuleConfig } type LineRuleParams = diff --git a/src/FSharpLint.Core/Rules/Conventions/Naming/AsynchronousFunctionNames.fs b/src/FSharpLint.Core/Rules/Conventions/Naming/AsynchronousFunctionNames.fs index e95c008ea..b697c04ec 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Naming/AsynchronousFunctionNames.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Naming/AsynchronousFunctionNames.fs @@ -32,8 +32,8 @@ let runner (config: Config) (args: AstNodeRuleParams) = | _ -> config.Mode = AllAPIs let likelyhoodOfBeingInLibrary = - match args.ProjectOptions.Value with - | Some projectOptions -> howLikelyProjectIsLibrary projectOptions.ProjectFileName + match args.ProjectFileName.Value with + | Some projectFileName -> howLikelyProjectIsLibrary projectFileName | None -> Unlikely if config.Mode = OnlyPublicAPIsInLibraries && likelyhoodOfBeingInLibrary <> Likely then diff --git a/src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs b/src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs index 27aa57f5a..71ba58bed 100644 --- a/src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs +++ b/src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs @@ -205,8 +205,8 @@ let runner (config: Config) (args: AstNodeRuleParams) = Array.append (checkFuncs asyncFuncs taskFuncs) (checkFuncs taskFuncs asyncFuncs) let likelyhoodOfBeingInLibrary = - match args.ProjectOptions.Value with - | Some projectOptions -> howLikelyProjectIsLibrary projectOptions.ProjectFileName + match args.ProjectFileName.Value with + | Some projectFileName -> howLikelyProjectIsLibrary projectFileName | None -> Unlikely if config.Mode = OnlyPublicAPIsInLibraries && likelyhoodOfBeingInLibrary <> Likely then diff --git a/src/FSharpLint.Core/Rules/Smells/NoAsyncRunSynchronouslyInLibrary.fs b/src/FSharpLint.Core/Rules/Smells/NoAsyncRunSynchronouslyInLibrary.fs index 2eab65600..3f242b7f2 100644 --- a/src/FSharpLint.Core/Rules/Smells/NoAsyncRunSynchronouslyInLibrary.fs +++ b/src/FSharpLint.Core/Rules/Smells/NoAsyncRunSynchronouslyInLibrary.fs @@ -95,9 +95,9 @@ let checkIfInLibrary (args: AstNodeRuleParams) (range: range) : array - let projectFile = System.IO.FileInfo projectOptions.ProjectFileName + match (args.CheckInfo, args.ProjectFileName.Value) with + | Some checkFileResults, Some projectFileName -> + let projectFile = System.IO.FileInfo projectFileName match howLikelyProjectIsLibrary projectFile.Name with | Likely -> false | Unlikely -> true diff --git a/tests/FSharpLint.Benchmarks/Benchmark.fs b/tests/FSharpLint.Benchmarks/Benchmark.fs index 361a6c04c..1798da984 100644 --- a/tests/FSharpLint.Benchmarks/Benchmark.fs +++ b/tests/FSharpLint.Benchmarks/Benchmark.fs @@ -30,7 +30,7 @@ type Benchmark () = let (fileInfo, _lines) = let text = File.ReadAllText sourceFile let tree = generateAst text sourceFile - ({ Ast = tree; Source = text; TypeCheckResults = None; ProjectCheckResults = None }, String.toLines text |> Array.toList) + ({ Ast = tree; Source = text; TypeCheckResults = None; ProjectCheckResults = None; ProjectFileName = None }, String.toLines text |> Array.toList) [] member this.LintParsedFile () = diff --git a/tests/FSharpLint.Core.Tests/Rules/TestAstNodeRule.fs b/tests/FSharpLint.Core.Tests/Rules/TestAstNodeRule.fs index ee77ae33b..c8dcb0a9e 100644 --- a/tests/FSharpLint.Core.Tests/Rules/TestAstNodeRule.fs +++ b/tests/FSharpLint.Core.Tests/Rules/TestAstNodeRule.fs @@ -43,7 +43,7 @@ type TestAstNodeRuleBase (rule:Rule) = GlobalConfig = resolvedGlobalConfig TypeCheckResults = checkResult ProjectCheckResults = None - ProjectOptions = Lazy<_>(None) + ProjectFileName = Lazy<_>(None) FilePath = (Option.defaultValue String.Empty maybeFileName) FileContent = input Lines = (input.Split("\n")) diff --git a/tests/FSharpLint.Core.Tests/Rules/TestHintMatcherBase.fs b/tests/FSharpLint.Core.Tests/Rules/TestHintMatcherBase.fs index fd52c56f4..9876b3e29 100644 --- a/tests/FSharpLint.Core.Tests/Rules/TestHintMatcherBase.fs +++ b/tests/FSharpLint.Core.Tests/Rules/TestHintMatcherBase.fs @@ -65,7 +65,7 @@ type TestHintMatcherBase () = GlobalConfig = resolvedGlobalConfig TypeCheckResults = checkResult ProjectCheckResults = None - ProjectOptions = Lazy<_>() + ProjectFileName = Lazy<_>() FilePath = (Option.defaultValue String.Empty maybeFileName) FileContent = input Lines = (input.Split("\n")) diff --git a/tests/FSharpLint.Core.Tests/Rules/TestIndentationRule.fs b/tests/FSharpLint.Core.Tests/Rules/TestIndentationRule.fs index 20c94fac0..9883f6d71 100644 --- a/tests/FSharpLint.Core.Tests/Rules/TestIndentationRule.fs +++ b/tests/FSharpLint.Core.Tests/Rules/TestIndentationRule.fs @@ -38,7 +38,7 @@ type TestIndentationRuleBase (rule:Rule) = GlobalConfig = resolvedGlobalConfig TypeCheckResults = None ProjectCheckResults = None - ProjectOptions = Lazy<_>(None) + ProjectFileName = Lazy<_>(None) FilePath = resolvedFileName FileContent = input Lines = lines diff --git a/tests/FSharpLint.Core.Tests/Rules/TestLineRule.fs b/tests/FSharpLint.Core.Tests/Rules/TestLineRule.fs index c53e93377..678ec8588 100644 --- a/tests/FSharpLint.Core.Tests/Rules/TestLineRule.fs +++ b/tests/FSharpLint.Core.Tests/Rules/TestLineRule.fs @@ -38,7 +38,7 @@ type TestLineRuleBase (rule:Rule) = GlobalConfig = resolvedGlobalConfig TypeCheckResults = None ProjectCheckResults = None - ProjectOptions = Lazy<_>(None) + ProjectFileName = Lazy<_>(None) FilePath = resolvedFileName FileContent = input Lines = lines diff --git a/tests/FSharpLint.Core.Tests/Rules/TestNoTabCharactersRule.fs b/tests/FSharpLint.Core.Tests/Rules/TestNoTabCharactersRule.fs index 5e534bb22..2e87e3b03 100644 --- a/tests/FSharpLint.Core.Tests/Rules/TestNoTabCharactersRule.fs +++ b/tests/FSharpLint.Core.Tests/Rules/TestNoTabCharactersRule.fs @@ -38,7 +38,7 @@ type TestNoTabCharactersRuleBase (rule:Rule) = GlobalConfig = resolvedGlobalConfig TypeCheckResults = None ProjectCheckResults = None - ProjectOptions = Lazy<_>() + ProjectFileName = Lazy<_>() FilePath = resolvedFileName FileContent = input Lines = lines diff --git a/tests/FSharpLint.FunctionalTest/TestApi.fs b/tests/FSharpLint.FunctionalTest/TestApi.fs index 635113f10..ef6c6611b 100644 --- a/tests/FSharpLint.FunctionalTest/TestApi.fs +++ b/tests/FSharpLint.FunctionalTest/TestApi.fs @@ -33,12 +33,28 @@ module TestApi = /// Must be called once per process. let toolsPath = Ionide.ProjInfo.Init.init (DirectoryInfo <| Directory.GetCurrentDirectory()) None + /// Parse + check `source` under FCS's TransparentCompiler (as an analyzer host would), + /// returning the parse tree together with the file and project check results. + let checkSourceUnderTransparentCompiler source = + let checker = FSharpChecker.Create(keepAssemblyContents = true, useTransparentCompiler = true) + let sourceText = SourceText.ofString source + let (options, _diagnostics) = + checker.GetProjectOptionsFromScript(sourceFile, sourceText) |> Async.RunSynchronously + let parseResults, checkAnswer = + checker.ParseAndCheckFileInProject(sourceFile, 0, sourceText, options) |> Async.RunSynchronously + let checkResults = + match checkAnswer with + | FSharpCheckFileAnswer.Succeeded results -> results + | FSharpCheckFileAnswer.Aborted -> failwith "type check aborted" + let projectResults = checker.ParseAndCheckProject options |> Async.RunSynchronously + (parseResults.ParseTree, checkResults, projectResults) + [] [] member _.``Performance of linting an existing file``() = let text = File.ReadAllText sourceFile let tree = generateAst text - let fileInfo = { Ast = tree; Source = text; TypeCheckResults = None; ProjectCheckResults = None } + let fileInfo = { Ast = tree; Source = text; TypeCheckResults = None; ProjectCheckResults = None; ProjectFileName = None } let stopwatch = Stopwatch.StartNew() let times = ResizeArray() @@ -59,6 +75,70 @@ module TestApi = Assert.Less(result, 250) fprintf TestContext.Out "Average runtime of linter on parsed file: %d (milliseconds)." result + /// Regression: analyzer hosts built on FCS's TransparentCompiler supply + /// ProjectCheckResults whose FSharpProjectContext.ProjectOptions getter throws by + /// design. Deriving the project file name from them must degrade to None (guarded) + /// rather than failing the whole file with an internal error. + [] + member _.``Lint parsed TransparentCompiler results without internal failure``() = + let source = "module TransparentCompilerRepro\n\nlet answer = async { return 42 }\n" + let parseTree, checkResults, projectResults = checkSourceUnderTransparentCompiler source + + let fileInfo = + { Ast = parseTree + Source = source + TypeCheckResults = Some checkResults + ProjectCheckResults = Some projectResults + ProjectFileName = None } + + let mutable internalFailure = None + let optionalParams = + { OptionalLintParameters.Default with + ReportLinterProgress = + Some (fun progress -> + match progress with + | ProjectProgress.Failed(_, ex) -> internalFailure <- Some ex + | _ -> ()) } + + match lintParsedFile optionalParams fileInfo sourceFile with + | LintResult.Success _ -> + match internalFailure with + | Some ex -> Assert.Fail $"lint failed internally: {ex}" + | None -> () + | LintResult.Failure failure -> Assert.Fail(string failure) + + /// The recovery this branch adds: a caller (e.g. an analyzer host) that knows the + /// project file supplies it directly via ProjectFileName, so the library-heuristic + /// rules keep working under the TransparentCompiler where the project options + /// cannot be derived from the check results. + [] + member _.``Caller-supplied ProjectFileName drives the library heuristic under TransparentCompiler``() = + // A public function returning Async<'T>: AsynchronousFunctionNames (default mode + // OnlyPublicAPIsInLibraries) flags it only when the project looks like a library. + let source = "module Foo =\n let Bar(): Async =\n async { return 1 }\n" + let parseTree, checkResults, projectResults = checkSourceUnderTransparentCompiler source + + let flagsBar projectFileName = + let fileInfo = + { Ast = parseTree + Source = source + TypeCheckResults = Some checkResults + ProjectCheckResults = Some projectResults + ProjectFileName = projectFileName } + match lintParsedFile OptionalLintParameters.Default fileInfo sourceFile with + | LintResult.Success warnings -> + warnings |> List.exists (fun warning -> warning.Details.Message.Contains "AsyncBar") + | LintResult.Failure failure -> failwith (string failure) + + // "MyLib.fsproj" tokenises to ["My"; "Lib"; ".fsproj"] -> Likely a library. + Assert.IsTrue( + flagsBar (Some "MyLib.fsproj"), + "caller-supplied library project name should engage the async-naming rule under TransparentCompiler") + // With no project file name the derivation is impossible under TC -> degrade to not-a-library. + Assert.IsFalse( + flagsBar None, + "with no project file name the library-gated rule should not fire") + [] member _.``Lint project via absolute path``() = let projectPath = basePath "tests" "FSharpLint.FunctionalTest.TestedProject" "FSharpLint.FunctionalTest.TestedProject.NetCore"