Skip to content

Commit

Permalink
Fix duplicated error in diagnostics (#1929)
Browse files Browse the repository at this point in the history 
* fixed: DocumentDiagnosticAnalyzer.AnalyzeSemanticAsync returns parse errors but should not

* fix DocumentDiagnosticAnalyzer tests
  • Loading branch information
@vasily-kirichenko @KevinRansom
vasily-kirichenko authored and KevinRansom committed Dec 6, 2016
1 parent 1099a5e commit b975863
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 39 deletions.
102 changes: 72 additions & 30 deletions vsintegration/src/FSharp.Editor/DocumentDiagnosticAnalyzer.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ namespace Microsoft.VisualStudio.FSharp.Editor
open System
open System.Composition
open System.Collections.Immutable
open System.Collections.Generic
open System.Threading
open System.Threading.Tasks

Expand All @@ -20,6 +21,11 @@ open Microsoft.FSharp.Compiler.Range

open Microsoft.VisualStudio.FSharp.LanguageService

[<RequireQualifiedAccess>]
type internal DiagnosticsType =
| Syntax
| Semantic

[<DiagnosticAnalyzer(FSharpCommonConstants.FSharpLanguageName)>]
type internal FSharpDocumentDiagnosticAnalyzer() =
inherit DocumentDiagnosticAnalyzer()
Expand All @@ -29,35 +35,71 @@ type internal FSharpDocumentDiagnosticAnalyzer() =

let getProjectInfoManager(document: Document) =
document.Project.Solution.Workspace.Services.GetService<FSharpCheckerWorkspaceService>().ProjectInfoManager

static let errorInfoEqualityComparer =
{ new IEqualityComparer<FSharpErrorInfo> with
member __.Equals (x, y) =
x.FileName = y.FileName &&
x.StartLineAlternate = y.StartLineAlternate &&
x.EndLineAlternate = y.EndLineAlternate &&
x.StartColumn = y.StartColumn &&
x.EndColumn = y.EndColumn &&
x.Severity = y.Severity &&
x.Message = y.Message &&
x.Subcategory = y.Subcategory &&
x.ErrorNumber = y.ErrorNumber
member __.GetHashCode x =
let mutable hash = 17
hash <- hash * 23 + x.EndLineAlternate.GetHashCode()
hash <- hash * 23 + x.EndLineAlternate.GetHashCode()
hash <- hash * 23 + x.StartColumn.GetHashCode()
hash <- hash * 23 + x.EndColumn.GetHashCode()
hash <- hash * 23 + x.Severity.GetHashCode()
hash <- hash * 23 + x.Message.GetHashCode()
hash <- hash * 23 + x.Subcategory.GetHashCode()
hash <- hash * 23 + x.ErrorNumber.GetHashCode()
hash
}

static member GetDiagnostics(checker: FSharpChecker, filePath: string, sourceText: SourceText, textVersionHash: int, options: FSharpProjectOptions, addSemanticErrors: bool) = async {
let! parseResults = checker.ParseFileInProject(filePath, sourceText.ToString(), options)
let! errors = async {
if addSemanticErrors then
let! checkResultsAnswer = checker.CheckFileInProject(parseResults, filePath, textVersionHash, sourceText.ToString(), options)
match checkResultsAnswer with
| FSharpCheckFileAnswer.Aborted -> return [| |]
| FSharpCheckFileAnswer.Succeeded(results) -> return results.Errors
else
return parseResults.Errors
}

let results =
(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(max 0 (sourceText.Length - 1), sourceText.Length)
let location = Location.Create(filePath, correctedTextSpan , linePositionSpan)
Some(CommonRoslynHelpers.ConvertError(error, location)))
).ToImmutableArray()
return results
}
static member GetDiagnostics(checker: FSharpChecker, filePath: string, sourceText: SourceText, textVersionHash: int, options: FSharpProjectOptions, diagnosticType: DiagnosticsType) =
async {
let! parseResults = checker.ParseFileInProject(filePath, sourceText.ToString(), options)
let! errors =
async {
match diagnosticType with
| DiagnosticsType.Semantic ->
let! checkResultsAnswer = checker.CheckFileInProject(parseResults, filePath, textVersionHash, sourceText.ToString(), options)
match checkResultsAnswer with
| FSharpCheckFileAnswer.Aborted -> return [||]
| FSharpCheckFileAnswer.Succeeded results ->
// In order to eleminate duplicates, we should not return parse errors here because they are returned by `AnalyzeSyntaxAsync` method.
let allErrors = HashSet(results.Errors, errorInfoEqualityComparer)
allErrors.ExceptWith(parseResults.Errors)
return Seq.toArray allErrors
| DiagnosticsType.Syntax ->
return parseResults.Errors
}

let results =
(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(max 0 (sourceText.Length - 1), sourceText.Length)

let location = Location.Create(filePath, correctedTextSpan , linePositionSpan)
Some(CommonRoslynHelpers.ConvertError(error, location)))
).ToImmutableArray()
return results
}

override this.SupportedDiagnostics = CommonRoslynHelpers.SupportedDiagnostics()

Expand All @@ -68,7 +110,7 @@ type internal FSharpDocumentDiagnosticAnalyzer() =
| Some options ->
let! sourceText = document.GetTextAsync(cancellationToken) |> Async.AwaitTask
let! textVersion = document.GetTextVersionAsync(cancellationToken) |> Async.AwaitTask
return! FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(getChecker document, document.FilePath, sourceText, textVersion.GetHashCode(), options, false)
return! FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(getChecker document, document.FilePath, sourceText, textVersion.GetHashCode(), options, DiagnosticsType.Syntax)
| None -> return ImmutableArray<Diagnostic>.Empty
} |> CommonRoslynHelpers.StartAsyncAsTask cancellationToken

Expand All @@ -81,7 +123,7 @@ type internal FSharpDocumentDiagnosticAnalyzer() =
| Some options ->
let! sourceText = document.GetTextAsync(cancellationToken) |> Async.AwaitTask
let! textVersion = document.GetTextVersionAsync(cancellationToken) |> Async.AwaitTask
return! FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(getChecker document, document.FilePath, sourceText, textVersion.GetHashCode(), options, true)
return! FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(getChecker document, document.FilePath, sourceText, textVersion.GetHashCode(), options, DiagnosticsType.Semantic)
| None -> return ImmutableArray<Diagnostic>.Empty
} |> CommonRoslynHelpers.StartAsyncAsTask cancellationToken

19 changes: 10 additions & 9 deletions vsintegration/tests/unittests/DocumentDiagnosticAnalyzerTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,23 @@ type DocumentDiagnosticAnalyzerTests() =
ExtraProjectInfo = None
}

let getDiagnostics (fileContents: string) =
async {
let! syntacticDiagnostics = FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(FSharpChecker.Instance, filePath, SourceText.From(fileContents), 0, options, DiagnosticsType.Syntax)
let! semanticDiagnostics = FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(FSharpChecker.Instance, filePath, SourceText.From(fileContents), 0, options, DiagnosticsType.Semantic)
return syntacticDiagnostics.AddRange(semanticDiagnostics)
} |> Async.RunSynchronously

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(FSharpChecker.Instance, filePath, SourceText.From(fileContents), 0, additionalOptions, true) |> Async.RunSynchronously
let errors = getDiagnostics fileContents
Assert.AreEqual(0, errors.Length, "There should be no errors generated")

member private this.VerifyErrorAtMarker(fileContents: string, expectedMarker: string, ?expectedMessage: string) =
let errors =
FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(FSharpChecker.Instance, filePath, SourceText.From(fileContents), 0, options, true)
|> Async.RunSynchronously
|> Seq.filter(fun e -> e.Severity = DiagnosticSeverity.Error) |> Seq.toArray
let errors = getDiagnostics fileContents |> Seq.filter(fun e -> e.Severity = DiagnosticSeverity.Error) |> Seq.toArray
Assert.AreEqual(1, errors.Length, "There should be exactly one error generated")
let actualError = errors.[0]
if expectedMessage.IsSome then
Expand All @@ -58,10 +62,7 @@ type DocumentDiagnosticAnalyzerTests() =
Assert.AreEqual(expectedEnd, actualError.Location.SourceSpan.End, "Error end positions should match")

member private this.VerifyDiagnosticBetweenMarkers(fileContents: string, expectedMessage: string, expectedSeverity: DiagnosticSeverity) =
let errors =
FSharpDocumentDiagnosticAnalyzer.GetDiagnostics(FSharpChecker.Instance, filePath, SourceText.From(fileContents), 0, options, true)
|> Async.RunSynchronously
|> Seq.filter(fun e -> e.Severity = expectedSeverity) |> Seq.toArray
let errors = getDiagnostics fileContents |> Seq.filter(fun e -> e.Severity = expectedSeverity) |> Seq.toArray
Assert.AreEqual(1, errors.Length, "There should be exactly one error generated")
let actualError = errors.[0]
Assert.AreEqual(expectedSeverity, actualError.Severity)
Expand Down

0 comments on commit b975863

Please sign in to comment.