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"