Skip to content

Commit

Permalink
Don't suggest names for errors in normal IDE usage (#6063)
Browse files Browse the repository at this point in the history
* Don't suggest names for errors in normal IDE usage

* Remove flag from parsing and project opens

* Move flag into FSharpChecker

* Suggest names based on symbols in the current document

* Cleanup and turn on code fix by default

* Use declarationlistinfo as a source for suggesting names

* Reduce diff
  • Loading branch information
cartermp authored Feb 5, 2019
1 parent caf462c commit da89b7f
Show file tree
Hide file tree
Showing 39 changed files with 295 additions and 140 deletions.
31 changes: 17 additions & 14 deletions src/fsharp/CompileOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ let getErrorString key = SR.GetString key

let (|InvalidArgument|_|) (exn:exn) = match exn with :? ArgumentException as e -> Some e.Message | _ -> None

let OutputPhasedErrorR (os:StringBuilder) (err:PhasedDiagnostic) =
let OutputPhasedErrorR (os:StringBuilder) (err:PhasedDiagnostic) (suggestNames: bool) =

let rec OutputExceptionR (os:StringBuilder) error =

Expand Down Expand Up @@ -827,9 +827,10 @@ let OutputPhasedErrorR (os:StringBuilder) (err:PhasedDiagnostic) =

| UndefinedName(_, k, id, suggestionsF) ->
os.Append(k (DecompileOpName id.idText)) |> ignore
let filtered = ErrorResolutionHints.FilterPredictions id.idText suggestionsF
if List.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore
if suggestNames then
let filtered = ErrorResolutionHints.FilterPredictions suggestionsF id.idText
if List.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore


| InternalUndefinedItemRef(f, smr, ccuName, s) ->
Expand Down Expand Up @@ -1367,9 +1368,10 @@ let OutputPhasedErrorR (os:StringBuilder) (err:PhasedDiagnostic) =

| ErrorWithSuggestions ((_, s), _, idText, suggestionF) ->
os.Append(DecompileOpName s) |> ignore
let filtered = ErrorResolutionHints.FilterPredictions idText suggestionF
if List.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore
if suggestNames then
let filtered = ErrorResolutionHints.FilterPredictions suggestionF idText
if List.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore

| NumberedError ((_, s), _) -> os.Append(s) |> ignore

Expand Down Expand Up @@ -1585,10 +1587,10 @@ let OutputPhasedErrorR (os:StringBuilder) (err:PhasedDiagnostic) =


// remove any newlines and tabs
let OutputPhasedDiagnostic (os:System.Text.StringBuilder) (err:PhasedDiagnostic) (flattenErrors:bool) =
let OutputPhasedDiagnostic (os:System.Text.StringBuilder) (err:PhasedDiagnostic) (flattenErrors:bool) (suggestNames: bool) =
let buf = new System.Text.StringBuilder()

OutputPhasedErrorR buf err
OutputPhasedErrorR buf err suggestNames
let s = if flattenErrors then ErrorLogger.NormalizeErrorString (buf.ToString()) else buf.ToString()

os.Append(s) |> ignore
Expand Down Expand Up @@ -1638,7 +1640,7 @@ type Diagnostic =
| Long of bool * DiagnosticDetailedInfo

/// returns sequence that contains Diagnostic for the given error + Diagnostic for all related errors
let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err:PhasedDiagnostic) =
let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err:PhasedDiagnostic, suggestNames: bool) =
let outputWhere (showFullPaths, errorStyle) m : DiagnosticLocation =
if Range.equals m rangeStartup || Range.equals m rangeCmdArgs then
{ Range = m; TextRepresentation = ""; IsEmpty = true; File = "" }
Expand Down Expand Up @@ -1711,7 +1713,7 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
let canonical = OutputCanonicalInformation(err.Subcategory(), GetDiagnosticNumber mainError)
let message =
let os = System.Text.StringBuilder()
OutputPhasedDiagnostic os mainError flattenErrors
OutputPhasedDiagnostic os mainError flattenErrors suggestNames
os.ToString()

let entry : DiagnosticDetailedInfo = { Location = where; Canonical = canonical; Message = message }
Expand All @@ -1726,15 +1728,15 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
let relCanonical = OutputCanonicalInformation(err.Subcategory(), GetDiagnosticNumber mainError) // Use main error for code
let relMessage =
let os = System.Text.StringBuilder()
OutputPhasedDiagnostic os err flattenErrors
OutputPhasedDiagnostic os err flattenErrors suggestNames
os.ToString()

let entry : DiagnosticDetailedInfo = { Location = relWhere; Canonical = relCanonical; Message = relMessage}
errors.Add( Diagnostic.Long (isError, entry) )

| _ ->
let os = System.Text.StringBuilder()
OutputPhasedDiagnostic os err flattenErrors
OutputPhasedDiagnostic os err flattenErrors suggestNames
errors.Add( Diagnostic.Short(isError, os.ToString()) )

relatedErrors |> List.iter OutputRelatedError
Expand All @@ -1755,7 +1757,8 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
/// prints error and related errors to the specified StringBuilder
let rec OutputDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError) os (err:PhasedDiagnostic) =

let errors = CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err)
// 'true' for "suggestNames" is passed last here because we want to report suggestions in fsc.exe and fsi.exe, just not in regular IDE usage.
let errors = CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err, true)
for e in errors do
Printf.bprintf os "\n"
match e with
Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/CompileOps.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ val GetDiagnosticNumber: PhasedDiagnostic -> int
val SplitRelatedDiagnostics: PhasedDiagnostic -> PhasedDiagnostic * PhasedDiagnostic list

/// Output an error to a buffer
val OutputPhasedDiagnostic: StringBuilder -> PhasedDiagnostic -> flattenErrors: bool -> unit
val OutputPhasedDiagnostic: StringBuilder -> PhasedDiagnostic -> flattenErrors: bool -> suggestNames: bool -> unit

/// Output an error or warning to a buffer
val OutputDiagnostic: implicitIncludeDir:string * showFullPaths: bool * flattenErrors: bool * errorStyle: ErrorStyle * isError:bool -> StringBuilder -> PhasedDiagnostic -> unit
Expand Down Expand Up @@ -120,7 +120,7 @@ type Diagnostic =
| Long of bool * DiagnosticDetailedInfo

/// Part of LegacyHostedCompilerForTesting
val CollectDiagnostic: implicitIncludeDir:string * showFullPaths: bool * flattenErrors: bool * errorStyle: ErrorStyle * warning:bool * PhasedDiagnostic -> seq<Diagnostic>
val CollectDiagnostic: implicitIncludeDir:string * showFullPaths: bool * flattenErrors: bool * errorStyle: ErrorStyle * warning:bool * PhasedDiagnostic * suggestNames: bool -> seq<Diagnostic>

//----------------------------------------------------------------------------
// Resolve assembly references
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/ErrorResolutionHints.fs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ let IsInEditDistanceProximity idText suggestion =
editDistance <= threshold

/// Filters predictions based on edit distance to the given unknown identifier.
let FilterPredictions (idText:string) (suggestionF:ErrorLogger.Suggestions) =
let FilterPredictions (suggestionF:ErrorLogger.Suggestions) (idText:string) =
let uppercaseText = idText.ToUpperInvariant()
let allSuggestions = suggestionF()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,12 @@
<Compile Include="..\service\service.fs">
<Link>Service/service.fs</Link>
</Compile>
<Compile Include="..\service\ServiceErrorResolutionHints.fsi">
<Link>Service/ServiceErrorResolutionHints.fsi</Link>
</Compile>
<Compile Include="..\service\ServiceErrorResolutionHints.fs">
<Link>Service/ServiceErrorResolutionHints.fs</Link>
</Compile>
<Compile Include="..\service\ServiceInterfaceStubGenerator.fsi">
<Link>Service/ServiceInterfaceStubGenerator.fsi</Link>
</Compile>
Expand Down
5 changes: 3 additions & 2 deletions src/fsharp/fsc.fs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ type InProcErrorLoggerProvider() =
member log.CreateErrorLoggerUpToMaxErrors(tcConfigBuilder, exiter) =
{ new ErrorLoggerUpToMaxErrors(tcConfigBuilder, exiter, "InProcCompilerErrorLoggerUpToMaxErrors") with
member this.HandleTooManyErrors(text) = warnings.Add(Diagnostic.Short(false, text))
member this.HandleIssue(tcConfigBuilder, err, isError) =
let errs = CollectDiagnostic(tcConfigBuilder.implicitIncludeDir, tcConfigBuilder.showFullPaths, tcConfigBuilder.flatErrors, tcConfigBuilder.errorStyle, isError, err)
member this.HandleIssue(tcConfigBuilder, err, isError) =
// 'true' is passed for "suggestNames", since we want to suggest names with fsc.exe runs and this doesn't affect IDE perf
let errs = CollectDiagnostic(tcConfigBuilder.implicitIncludeDir, tcConfigBuilder.showFullPaths, tcConfigBuilder.flatErrors, tcConfigBuilder.errorStyle, isError, err, true)
let container = if isError then errors else warnings
container.AddRange(errs) }
:> ErrorLogger }
Expand Down
3 changes: 2 additions & 1 deletion src/fsharp/fsi/fsi.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2594,7 +2594,8 @@ type FsiEvaluationSession (fsi: FsiEvaluationSessionHostConfig, argv:string[], i
| Choice2Of2 None -> Choice2Of2 (System.Exception "Operation could not be completed due to earlier error")
| Choice2Of2 (Some userExn) -> Choice2Of2 userExn

userRes, ErrorHelpers.CreateErrorInfos (errorOptions, true, scriptFile, errs)
// 'true' is passed for "suggestNames" because we want the FSI session to suggest names for misspellings and it won't affect IDE perf much
userRes, ErrorHelpers.CreateErrorInfos (errorOptions, true, scriptFile, errs, true)

let dummyScriptFileName = "input.fsx"

Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/service/IncrementalBuild.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1741,7 +1741,7 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput

/// CreateIncrementalBuilder (for background type checking). Note that fsc.fs also
/// creates an incremental builder used by the command line compiler.
static member TryCreateBackgroundBuilderForProjectOptions (ctok, legacyReferenceResolver, defaultFSharpBinariesDir, frameworkTcImportsCache: FrameworkImportsCache, loadClosureOpt:LoadClosure option, sourceFiles:string list, commandLineArgs:string list, projectReferences, projectDirectory, useScriptResolutionRules, keepAssemblyContents, keepAllBackgroundResolutions, maxTimeShareMilliseconds, tryGetMetadataSnapshot) =
static member TryCreateBackgroundBuilderForProjectOptions (ctok, legacyReferenceResolver, defaultFSharpBinariesDir, frameworkTcImportsCache: FrameworkImportsCache, loadClosureOpt:LoadClosure option, sourceFiles:string list, commandLineArgs:string list, projectReferences, projectDirectory, useScriptResolutionRules, keepAssemblyContents, keepAllBackgroundResolutions, maxTimeShareMilliseconds, tryGetMetadataSnapshot, suggestNamesForErrors) =
let useSimpleResolutionSwitch = "--simpleresolution"

cancellable {
Expand Down Expand Up @@ -1874,7 +1874,7 @@ type IncrementalBuilder(tcGlobals, frameworkTcImports, nonFrameworkAssemblyInput
errorLogger.GetErrors() |> Array.map (fun (d, severity) -> d, severity = FSharpErrorSeverity.Error)
| _ ->
Array.ofList delayedLogger.Diagnostics
|> Array.map (fun (d, isError) -> FSharpErrorInfo.CreateFromException(d, isError, range.Zero))
|> Array.map (fun (d, isError) -> FSharpErrorInfo.CreateFromException(d, isError, range.Zero, suggestNamesForErrors))

return builderOpt, diagnostics
}
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/service/IncrementalBuild.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ type internal IncrementalBuilder =
/// This may be a marginally long-running operation (parses are relatively quick, only one file needs to be parsed)
member GetParseResultsForFile : CompilationThreadToken * filename:string -> Cancellable<Ast.ParsedInput option * Range.range * string * (PhasedDiagnostic * FSharpErrorSeverity)[]>

static member TryCreateBackgroundBuilderForProjectOptions : CompilationThreadToken * ReferenceResolver.Resolver * defaultFSharpBinariesDir: string * FrameworkImportsCache * scriptClosureOptions:LoadClosure option * sourceFiles:string list * commandLineArgs:string list * projectReferences: IProjectReference list * projectDirectory:string * useScriptResolutionRules:bool * keepAssemblyContents: bool * keepAllBackgroundResolutions: bool * maxTimeShareMilliseconds: int64 * tryGetMetadataSnapshot: ILBinaryReader.ILReaderTryGetMetadataSnapshot -> Cancellable<IncrementalBuilder option * FSharpErrorInfo[]>
static member TryCreateBackgroundBuilderForProjectOptions : CompilationThreadToken * ReferenceResolver.Resolver * defaultFSharpBinariesDir: string * FrameworkImportsCache * scriptClosureOptions:LoadClosure option * sourceFiles:string list * commandLineArgs:string list * projectReferences: IProjectReference list * projectDirectory:string * useScriptResolutionRules:bool * keepAssemblyContents: bool * keepAllBackgroundResolutions: bool * maxTimeShareMilliseconds: int64 * tryGetMetadataSnapshot: ILBinaryReader.ILReaderTryGetMetadataSnapshot * suggestNamesForErrors: bool -> Cancellable<IncrementalBuilder option * FSharpErrorInfo[]>

/// Increment the usage count on the IncrementalBuilder by 1. This initial usage count is 0 so immediately after creation
/// a call to KeepBuilderAlive should be made. The returns an IDisposable which will
Expand Down
14 changes: 14 additions & 0 deletions src/fsharp/service/ServiceErrorResolutionHints.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

namespace Microsoft.FSharp.Compiler.SourceCodeServices

open System.Collections.Generic

open Microsoft.FSharp.Compiler.ErrorResolutionHints

module ErrorResolutionHints =
let getSuggestedNames (namesToCheck: string[]) (unresolvedIdentifier: string) =
let res = FilterPredictions (fun () -> HashSet<string>(namesToCheck)) unresolvedIdentifier |> List.map snd
match res with
| [] -> None
| _ -> Some res
9 changes: 9 additions & 0 deletions src/fsharp/service/ServiceErrorResolutionHints.fsi
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

namespace Microsoft.FSharp.Compiler.SourceCodeServices

/// Exposes the string distance algorithm used to suggest names for mistyped identifiers.
module ErrorResolutionHints =
/// Given a set of names, uses and a string representing an unresolved identifier,
/// returns a list of suggested names if there are any feasible candidates.
val getSuggestedNames: symbolUses: string[] -> unresolvedIdentifier: string -> string list option
Loading

0 comments on commit da89b7f

Please sign in to comment.