From 6d79320c7517c4af623886dfb107d2edf1d3f43a Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Fri, 14 Oct 2016 18:30:04 -0700 Subject: [PATCH 1/2] Added ProjectDiagnosticAnalyzer --- .../src/FSharp.Editor/CommonRoslynHelpers.fs | 15 ++++++ .../DocumentDiagnosticAnalyzer.fs | 42 ++++++---------- .../src/FSharp.Editor/FSharp.Editor.fsproj | 3 ++ .../ProjectDiagnosticAnalyzer.fs | 49 +++++++++++++++++++ 4 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 vsintegration/src/FSharp.Editor/ProjectDiagnosticAnalyzer.fs diff --git a/vsintegration/src/FSharp.Editor/CommonRoslynHelpers.fs b/vsintegration/src/FSharp.Editor/CommonRoslynHelpers.fs index f0ffc238726..13bdda569c5 100644 --- a/vsintegration/src/FSharp.Editor/CommonRoslynHelpers.fs +++ b/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(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) diff --git a/vsintegration/src/FSharp.Editor/DocumentDiagnosticAnalyzer.fs b/vsintegration/src/FSharp.Editor/DocumentDiagnosticAnalyzer.fs index 2c152f0760c..6eeb31ef073 100644 --- a/vsintegration/src/FSharp.Editor/DocumentDiagnosticAnalyzer.fs +++ b/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(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> = let computation = async { diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index 3e42e9cf8c0..5fab2a99af1 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -51,6 +51,9 @@ Diagnostics\DocumentDiagnosticAnalyzer.fs + + Diagnostics\ProjectDiagnosticAnalyzer.fs + Completion\CompletionProvider.fs diff --git a/vsintegration/src/FSharp.Editor/ProjectDiagnosticAnalyzer.fs b/vsintegration/src/FSharp.Editor/ProjectDiagnosticAnalyzer.fs new file mode 100644 index 00000000000..df1e56d07a7 --- /dev/null +++ b/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 + +[] +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> = + let computation = async { + match FSharpLanguageService.GetOptions(project.Id) with + | Some(options) -> + return FSharpProjectDiagnosticAnalyzer.GetDiagnostics(options) + | None -> return ImmutableArray.Empty + } + + Async.StartAsTask(computation, TaskCreationOptions.None, cancellationToken) + .ContinueWith(CommonRoslynHelpers.GetCompletedTaskResult, cancellationToken) From e8a69c0cc7d41e8edefa835bf52b6a505c5e7971 Mon Sep 17 00:00:00 2001 From: Omar Tawfik Date: Tue, 18 Oct 2016 14:14:33 -0700 Subject: [PATCH 2/2] Added tests --- .../DocumentDiagnosticAnalyzerTests.fs | 18 +++++- .../ProjectDiagnosticAnalyzerTests.fs | 63 +++++++++++++++++++ .../unittests/VisualFSharp.Unittests.fsproj | 51 ++++++++------- 3 files changed, 106 insertions(+), 26 deletions(-) create mode 100644 vsintegration/tests/unittests/ProjectDiagnosticAnalyzerTests.fs diff --git a/vsintegration/tests/unittests/DocumentDiagnosticAnalyzerTests.fs b/vsintegration/tests/unittests/DocumentDiagnosticAnalyzerTests.fs index 0b690a3c6f7..165a5c88cb5 100644 --- a/vsintegration/tests/unittests/DocumentDiagnosticAnalyzerTests.fs +++ b/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() """) + + [] + member public this.DocumentDiagnosticsDontReportProjectErrors_Bug1596() = + // https://github.com/Microsoft/visualfsharp/issues/1596 + this.VerifyNoErrors( + fileContents = """ +let x = 3 +printf "%d" x + """, + additionalFlags = [| "--times" |]) diff --git a/vsintegration/tests/unittests/ProjectDiagnosticAnalyzerTests.fs b/vsintegration/tests/unittests/ProjectDiagnosticAnalyzerTests.fs new file mode 100644 index 00000000000..7753c6d1c5a --- /dev/null +++ b/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 + +[] +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) + + [] + 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()) + + [] + 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") diff --git a/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj b/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj index 38a515cc607..0be77892091 100644 --- a/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj +++ b/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj @@ -34,30 +34,6 @@ - - Roslyn\Classification\ColorizationServiceTests.fs - - - Roslyn\Utilities\BraceMatchingServiceTests.fs - - - Roslyn\Utilities\IndentationServiceTests.fs - - - Roslyn\Debugging\BreakpointResolutionService.fs - - - Roslyn\Debugging\LanguageDebugInfoServiceTests.fs - - - Roslyn\Diagnostics\DocumentDiagnosticAnalyzerTests.fs - - - Roslyn\Completion\CompletionProviderTests.fs - - - Roslyn\GoToDefinition\GoToDefinitionServiceTests.fs - @@ -105,6 +81,33 @@ ProjectOptionsTests.fs + + Roslyn\Classification\ColorizationServiceTests.fs + + + Roslyn\Utilities\BraceMatchingServiceTests.fs + + + Roslyn\Utilities\IndentationServiceTests.fs + + + Roslyn\Debugging\BreakpointResolutionService.fs + + + Roslyn\Debugging\LanguageDebugInfoServiceTests.fs + + + Roslyn\Diagnostics\DocumentDiagnosticAnalyzerTests.fs + + + Roslyn\Diagnostics\ProjectDiagnosticAnalyzerTests.fs + + + Roslyn\Completion\CompletionProviderTests.fs + + + Roslyn\GoToDefinition\GoToDefinitionServiceTests.fs + VisualFSharp.Unittests.dll.config