Separate Project/Document diagnostics

vsintegration/src/FSharp.Editor/CommonRoslynHelpers.fs

@@ -3,9 +3,11 @@
 namespace Microsoft.VisualStudio.FSharp.Editor
 
 open System
+open System.Collections.Immutable
 open System.Threading.Tasks
 open Microsoft.CodeAnalysis
 open Microsoft.CodeAnalysis.Text
+open Microsoft.FSharp.Compiler
 open Microsoft.FSharp.Compiler.SourceCodeServices
 open Microsoft.FSharp.Compiler.Range
 open Microsoft.VisualStudio.FSharp.LanguageService
@@ -34,3 +36,16 @@ module internal CommonRoslynHelpers =
         else
             Assert.Exception(task.Exception.GetBaseException())
             raise(task.Exception.GetBaseException())
+
+    let SupportedDiagnostics() =
+        // We are constructing our own descriptors at run-time. Compiler service is already doing error formatting and localization.
+        let dummyDescriptor = DiagnosticDescriptor("0", String.Empty, String.Empty, String.Empty, DiagnosticSeverity.Error, true, null, null)
+        ImmutableArray.Create<DiagnosticDescriptor>(dummyDescriptor)
+
+    let ConvertError(error: FSharpErrorInfo, location: Location) =
+        let id = "FS" + error.ErrorNumber.ToString("0000")
+        let emptyString = LocalizableString.op_Implicit("")
+        let description = LocalizableString.op_Implicit(error.Message)
+        let severity = if error.Severity = FSharpErrorSeverity.Error then DiagnosticSeverity.Error else DiagnosticSeverity.Warning
+        let descriptor = new DiagnosticDescriptor(id, emptyString, description, error.Subcategory, severity, true, emptyString, String.Empty, null)
+        Diagnostic.Create(descriptor, location)

vsintegration/src/FSharp.Editor/DocumentDiagnosticAnalyzer.fs

@@ -24,22 +24,6 @@ open Microsoft.VisualStudio.FSharp.LanguageService
 type internal FSharpDocumentDiagnosticAnalyzer() =
     inherit DocumentDiagnosticAnalyzer()
 
-    static member ConvertError(filePath: string, sourceText: SourceText, error: FSharpErrorInfo) =
-        let id = "FS" + error.ErrorNumber.ToString("0000")
-        let emptyString = LocalizableString.op_Implicit("")
-        let description = LocalizableString.op_Implicit(error.Message)
-        let severity = if error.Severity = FSharpErrorSeverity.Error then DiagnosticSeverity.Error else DiagnosticSeverity.Warning
-        let descriptor = new DiagnosticDescriptor(id, emptyString, description, error.Subcategory, severity, true, emptyString, String.Empty, null)
-
-        // F# compiler report errors at start/end of file if parsing fails. It should be corrected to match Roslyn boundaries
-        let linePositionSpan = LinePositionSpan(
-                                LinePosition(Math.Max(0, error.StartLineAlternate - 1), error.StartColumn),
-                                LinePosition(Math.Max(0, error.EndLineAlternate - 1), error.EndColumn))
-        let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan)
-        let correctedTextSpan = if textSpan.End < sourceText.Length then textSpan else TextSpan.FromBounds(sourceText.Length - 1, sourceText.Length)
-
-        Diagnostic.Create(descriptor, Location.Create(filePath, correctedTextSpan , linePositionSpan))
-
     static member GetDiagnostics(filePath: string, sourceText: SourceText, textVersionHash: int, options: FSharpProjectOptions, addSemanticErrors: bool) =
         let parseResults = FSharpChecker.Instance.ParseFileInProject(filePath, sourceText.ToString(), options) |> Async.RunSynchronously
         let errors =
@@ -50,16 +34,22 @@ type internal FSharpDocumentDiagnosticAnalyzer() =
                 | FSharpCheckFileAnswer.Succeeded(results) -> results.Errors
             else
                 parseResults.Errors
-
-        (errors |> Seq.map(fun (error) -> FSharpDocumentDiagnosticAnalyzer.ConvertError(filePath, sourceText, error))).ToImmutableArray()
-
-
-    // We are constructing our own descriptors at run-time. Compiler service is already doing error formatting and localization.
-    override this.SupportedDiagnostics
-        with get() =
-            let dummyDescriptor = DiagnosticDescriptor("0", String.Empty, String.Empty, String.Empty, DiagnosticSeverity.Error, true, null, null)
-            ImmutableArray.Create<DiagnosticDescriptor>(dummyDescriptor)
-
+
+        (errors |> Seq.choose(fun (error) ->
+            if error.StartLineAlternate = 0 || error.EndLineAlternate = 0 then
+                // F# error line numbers are one-based. Compiler returns 0 for global errors (reported by ProjectDiagnosticAnalyzer)
+                None
+            else
+                // Roslyn line numbers are zero-based
+                let linePositionSpan = LinePositionSpan(LinePosition(error.StartLineAlternate - 1, error.StartColumn),LinePosition(error.EndLineAlternate - 1, error.EndColumn))
+                let textSpan = sourceText.Lines.GetTextSpan(linePositionSpan)
+                // F# compiler report errors at end of file if parsing fails. It should be corrected to match Roslyn boundaries
+                let correctedTextSpan = if textSpan.End < sourceText.Length then textSpan else TextSpan.FromBounds(sourceText.Length - 1, sourceText.Length)
+                let location = Location.Create(filePath, correctedTextSpan , linePositionSpan)
+                Some(CommonRoslynHelpers.ConvertError(error, location)))
+        ).ToImmutableArray()
+
+    override this.SupportedDiagnostics with get() = CommonRoslynHelpers.SupportedDiagnostics()
 
     override this.AnalyzeSyntaxAsync(document: Document, cancellationToken: CancellationToken): Task<ImmutableArray<Diagnostic>> =
         let computation = async {

vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj

@@ -51,6 +51,9 @@
     <Compile Include="DocumentDiagnosticAnalyzer.fs">
       <Link>Diagnostics\DocumentDiagnosticAnalyzer.fs</Link>
     </Compile>
+    <Compile Include="ProjectDiagnosticAnalyzer.fs">
+      <Link>Diagnostics\ProjectDiagnosticAnalyzer.fs</Link>
+    </Compile>
     <Compile Include="CompletionProvider.fs">
       <Link>Completion\CompletionProvider.fs</Link>
     </Compile>

vsintegration/src/FSharp.Editor/ProjectDiagnosticAnalyzer.fs

@@ -0,0 +1,49 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+namespace Microsoft.VisualStudio.FSharp.Editor
+
+open System
+open System.Composition
+open System.Collections.Immutable
+open System.IO
+open System.Threading
+open System.Threading.Tasks
+
+open Microsoft.CodeAnalysis
+open Microsoft.CodeAnalysis.Diagnostics
+open Microsoft.CodeAnalysis.Host.Mef
+open Microsoft.CodeAnalysis.Text
+open Microsoft.CodeAnalysis.SolutionCrawler
+
+open Microsoft.FSharp.Compiler
+open Microsoft.FSharp.Compiler.SourceCodeServices
+open Microsoft.FSharp.Compiler.Range
+
+open Microsoft.VisualStudio.FSharp.LanguageService
+
+[<DiagnosticAnalyzer(FSharpCommonConstants.FSharpLanguageName)>]
+type internal FSharpProjectDiagnosticAnalyzer() =
+    inherit ProjectDiagnosticAnalyzer()
+
+    static member GetDiagnostics(options: FSharpProjectOptions) =
+        let checkProjectResults = FSharpChecker.Instance.ParseAndCheckProject(options) |> Async.RunSynchronously
+        (checkProjectResults.Errors |> Seq.choose(fun (error) ->
+            if error.StartLineAlternate = 0 || error.EndLineAlternate = 0 then
+                Some(CommonRoslynHelpers.ConvertError(error, Location.None))
+            else
+                // F# error line numbers are one-based. Errors that have a valid line number are reported by DocumentDiagnosticAnalyzer
+                None
+        )).ToImmutableArray()
+
+    override this.SupportedDiagnostics with get() = CommonRoslynHelpers.SupportedDiagnostics()
+
+    override this.AnalyzeProjectAsync(project: Project, cancellationToken: CancellationToken): Task<ImmutableArray<Diagnostic>> =
+        let computation = async {
+            match FSharpLanguageService.GetOptions(project.Id) with
+            | Some(options) ->
+                return FSharpProjectDiagnosticAnalyzer.GetDiagnostics(options)
+            | None -> return ImmutableArray<Diagnostic>.Empty
+        }
+
+        Async.StartAsTask(computation, TaskCreationOptions.None, cancellationToken)
+             .ContinueWith(CommonRoslynHelpers.GetCompletedTaskResult, cancellationToken)

vsintegration/tests/unittests/DocumentDiagnosticAnalyzerTests.fs

@@ -33,8 +33,12 @@ type DocumentDiagnosticAnalyzerTests()  =
         UnresolvedReferences = None
     }
 
-    member private this.VerifyNoErrors(fileContents: string) =
-        let errors = FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(filePath, SourceText.From(fileContents), 0, options, true)
+    member private this.VerifyNoErrors(fileContents: string, ?additionalFlags: string[]) =
+        let additionalOptions = match additionalFlags with
+                                | None -> options
+                                | Some(flags) -> {options with OtherOptions = Array.append options.OtherOptions flags}
+
+        let errors = FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(filePath, SourceText.From(fileContents), 0, additionalOptions, true)
         Assert.AreEqual(0, errors.Length, "There should be no errors generated")
 
     member private this.VerifyErrorAtMarker(fileContents: string, expectedMarker: string, ?expectedMessage: string) =
@@ -372,3 +376,13 @@ type T =
 
 let g (t : T) = t.Count()
             """)
+
+    [<Test>]
+    member public this.DocumentDiagnosticsDontReportProjectErrors_Bug1596() =
+        // https://github.com/Microsoft/visualfsharp/issues/1596
+        this.VerifyNoErrors(
+            fileContents = """
+let x = 3
+printf "%d" x
+            """,
+            additionalFlags = [| "--times" |])

vsintegration/tests/unittests/ProjectDiagnosticAnalyzerTests.fs

@@ -0,0 +1,63 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+namespace Microsoft.VisualStudio.FSharp.Editor.Tests.Roslyn
+
+open System
+open System.IO
+open System.Threading
+
+open FSharp.Compiler.Service.Tests.Common
+
+open NUnit.Framework
+
+open Microsoft.CodeAnalysis
+open Microsoft.CodeAnalysis.Classification
+open Microsoft.CodeAnalysis.Editor
+open Microsoft.CodeAnalysis.Text
+
+open Microsoft.VisualStudio.FSharp.Editor
+open Microsoft.VisualStudio.FSharp.LanguageService
+
+open Microsoft.FSharp.Compiler.SourceCodeServices
+open Microsoft.FSharp.Compiler.Range
+
+[<TestFixture>]
+type ProjectDiagnosticAnalyzerTests()  =
+
+    let CreateProjectAndGetOptions(fileContents: string) =
+        let tempName = Path.GetTempFileName()
+        let fileName = Path.ChangeExtension(tempName, ".fs")
+        let projectName = Path.ChangeExtension(tempName, ".fsproj")
+        let dllName = Path.ChangeExtension(tempName, ".dll")
+        File.WriteAllText(fileName, fileContents)
+
+        let args = mkProjectCommandLineArgs (dllName, [fileName])
+        checker.GetProjectOptionsFromCommandLineArgs (projectName, args)
+
+    [<Test>]
+    member public this.ProjectDiagnosticsDontReportJustProjectErrors_Bug1596() =
+        // https://github.com/Microsoft/visualfsharp/issues/1596
+        let fileContents = """
+let x = 3
+printf "%d" x
+"""
+        let options = CreateProjectAndGetOptions(fileContents)
+        let additionalOptions = {options with OtherOptions = Array.append options.OtherOptions [| "--times" |]}
+
+        let errors = FSharpProjectDiagnosticAnalyzer.GetDiagnostics(additionalOptions)
+        Assert.AreEqual(1, errors.Length, "Exactly one warning should have been reported")
+
+        let warning = errors.[0]
+        Assert.AreEqual(DiagnosticSeverity.Warning, warning.Severity, "Diagnostic severity should be a warning")
+        Assert.AreEqual("The command-line option 'times' is for test purposes only", warning.GetMessage())
+
+    [<Test>]
+    member public this.ProjectDiagnosticsShouldNotReportDocumentErrors_Bug1596() =
+        // https://github.com/Microsoft/visualfsharp/issues/1596
+        let fileContents = """
+let x = "string value that cannot be printed with %d"
+printf "%d" x
+"""
+        let options = CreateProjectAndGetOptions(fileContents)
+
+        let errors = FSharpProjectDiagnosticAnalyzer.GetDiagnostics(options)
+        Assert.AreEqual(0, errors.Length, "No semantic errors should have been reported")

vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj

@@ -34,30 +34,6 @@
     <Compile Include="TestLib.Salsa.fs" />
     <Compile Include="TestLib.LanguageService.fs" />
     <Compile Include="TestLib.ProjectSystem.fs" />
-    <Compile Include="ColorizationServiceTests.fs">
-      <Link>Roslyn\Classification\ColorizationServiceTests.fs</Link>
-    </Compile>
-    <Compile Include="BraceMatchingServiceTests.fs">
-      <Link>Roslyn\Utilities\BraceMatchingServiceTests.fs</Link>
-    </Compile>
-    <Compile Include="IndentationServiceTests.fs">
-      <Link>Roslyn\Utilities\IndentationServiceTests.fs</Link>
-    </Compile>
-    <Compile Include="BreakpointResolutionService.fs">
-      <Link>Roslyn\Debugging\BreakpointResolutionService.fs</Link>
-    </Compile>
-    <Compile Include="LanguageDebugInfoServiceTests.fs">
-      <Link>Roslyn\Debugging\LanguageDebugInfoServiceTests.fs</Link>
-    </Compile>
-    <Compile Include="DocumentDiagnosticAnalyzerTests.fs">
-      <Link>Roslyn\Diagnostics\DocumentDiagnosticAnalyzerTests.fs</Link>
-    </Compile>
-    <Compile Include="CompletionProviderTests.fs">
-      <Link>Roslyn\Completion\CompletionProviderTests.fs</Link>
-    </Compile>
-    <Compile Include="GoToDefinitionServiceTests.fs">
-      <Link>Roslyn\GoToDefinition\GoToDefinitionServiceTests.fs</Link>
-    </Compile>
     <Compile Include="Tests.InternalCollections.fs" />
     <Compile Include="Tests.Build.fs" />
     <Compile Include="Tests.TaskReporter.fs" />
@@ -105,6 +81,33 @@
     <Compile Include="..\..\..\tests\service\ProjectOptionsTests.fs">
       <Link>ProjectOptionsTests.fs</Link>
     </Compile>
+    <Compile Include="ColorizationServiceTests.fs">
+      <Link>Roslyn\Classification\ColorizationServiceTests.fs</Link>
+    </Compile>
+    <Compile Include="BraceMatchingServiceTests.fs">
+      <Link>Roslyn\Utilities\BraceMatchingServiceTests.fs</Link>
+    </Compile>
+    <Compile Include="IndentationServiceTests.fs">
+      <Link>Roslyn\Utilities\IndentationServiceTests.fs</Link>
+    </Compile>
+    <Compile Include="BreakpointResolutionService.fs">
+      <Link>Roslyn\Debugging\BreakpointResolutionService.fs</Link>
+    </Compile>
+    <Compile Include="LanguageDebugInfoServiceTests.fs">
+      <Link>Roslyn\Debugging\LanguageDebugInfoServiceTests.fs</Link>
+    </Compile>
+    <Compile Include="DocumentDiagnosticAnalyzerTests.fs">
+      <Link>Roslyn\Diagnostics\DocumentDiagnosticAnalyzerTests.fs</Link>
+    </Compile>
+    <Compile Include="ProjectDiagnosticAnalyzerTests.fs">
+      <Link>Roslyn\Diagnostics\ProjectDiagnosticAnalyzerTests.fs</Link>
+    </Compile>
+    <Compile Include="CompletionProviderTests.fs">
+      <Link>Roslyn\Completion\CompletionProviderTests.fs</Link>
+    </Compile>
+    <Compile Include="GoToDefinitionServiceTests.fs">
+      <Link>Roslyn\GoToDefinition\GoToDefinitionServiceTests.fs</Link>
+    </Compile>
     <CustomCopyLocal Include="app.config">
       <TargetFilename>VisualFSharp.Unittests.dll.config</TargetFilename>
     </CustomCopyLocal>