Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a MaxBuffer for Suggestions #7060

Merged
merged 4 commits into from
Jun 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 24 additions & 18 deletions src/fsharp/CompileOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,20 @@ 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) (suggestNames: bool) =
let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNames: bool) =

let suggestNames suggestionsF idText =
if canSuggestNames then
let buffer = ErrorResolutionHints.SuggestionBuffer idText
if not buffer.Disabled then
suggestionsF buffer.Add
if not buffer.IsEmpty then
os.Append " " |> ignore
os.Append(FSComp.SR.undefinedNameSuggestionsIntro()) |> ignore
for value in buffer do
os.AppendLine() |> ignore
os.Append " " |> ignore
os.Append(DecompileOpName value) |> ignore

let rec OutputExceptionR (os: StringBuilder) error =

Expand Down Expand Up @@ -822,14 +835,10 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames

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

suggestNames suggestionsF id.idText

| InternalUndefinedItemRef(f, smr, ccuName, s) ->
let _, errs = f(smr, ccuName, s)
let _, errs = f(smr, ccuName, s)
os.Append errs |> ignore

| FieldNotMutable _ ->
Expand Down Expand Up @@ -1363,10 +1372,7 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames

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

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

Expand Down Expand Up @@ -1582,10 +1588,10 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames


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

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

os.Append s |> ignore
Expand Down Expand Up @@ -1635,7 +1641,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, suggestNames: bool) =
let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err: PhasedDiagnostic, canSuggestNames: 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 @@ -1708,7 +1714,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 suggestNames
OutputPhasedDiagnostic os mainError flattenErrors canSuggestNames
os.ToString()

let entry: DiagnosticDetailedInfo = { Location = where; Canonical = canonical; Message = message }
Expand All @@ -1723,15 +1729,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 suggestNames
OutputPhasedDiagnostic os err flattenErrors canSuggestNames
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 suggestNames
OutputPhasedDiagnostic os err flattenErrors canSuggestNames
errors.Add( Diagnostic.Short(isError, os.ToString()) )

relatedErrors |> List.iter OutputRelatedError
Expand All @@ -1752,7 +1758,7 @@ 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) =

// '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.
// 'true' for "canSuggestNames" 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"
Expand Down
9 changes: 4 additions & 5 deletions src/fsharp/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2193,12 +2193,11 @@ and ReportNoCandidatesError (csenv: ConstraintSolverEnv) (nUnnamedCallerArgs, nN
match cmeth.UnassignedNamedArgs with
| CallerNamedArg(id, _) :: _ ->
if minfo.IsConstructor then
let predictFields() =
minfo.DeclaringTyconRef.AllInstanceFieldsAsList
|> List.map (fun p -> p.Name.Replace("@", ""))
|> System.Collections.Generic.HashSet
let suggestFields (addToBuffer: string -> unit) =
for p in minfo.DeclaringTyconRef.AllInstanceFieldsAsList do
addToBuffer(p.Name.Replace("@", ""))

ErrorWithSuggestions((msgNum, FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange, id.idText, predictFields)
ErrorWithSuggestions((msgNum, FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange, id.idText, suggestFields)
else
Error((msgNum, FSComp.SR.csMemberHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange)
| [] -> Error((msgNum, msgText), m)
Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/ErrorLogger.fs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ let rec findOriginalException err =
| WrappedError(err, _) -> findOriginalException err
| _ -> err

type Suggestions = unit -> Collections.Generic.HashSet<string>
type Suggestions = (string -> unit) -> unit

let NoSuggestions : Suggestions = fun () -> Collections.Generic.HashSet()
let NoSuggestions : Suggestions = ignore

/// Thrown when we stop processing the F# Interactive entry or #load.
exception StopProcessingExn of exn option with
Expand Down
133 changes: 76 additions & 57 deletions src/fsharp/ErrorResolutionHints.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ module internal FSharp.Compiler.ErrorResolutionHints

open Internal.Utilities
open FSharp.Compiler.AbstractIL.Internal.Library
open System.Collections
open System.Collections.Generic

let maxSuggestions = 5
let minThresholdForSuggestions = 0.7
let highConfidenceThreshold = 0.85
let minStringLengthForThreshold = 3
let minStringLengthForSuggestion = 3

/// We report a candidate if its edit distance is <= the threshold.
/// The threshold is set to about a quarter of the number of characters.
Expand All @@ -23,65 +25,82 @@ let IsInEditDistanceProximity idText suggestion =

editDistance <= threshold

/// Filters predictions based on edit distance to the given unknown identifier.
let FilterPredictions (suggestionF:ErrorLogger.Suggestions) (idText:string) =
/// Demangles a suggestion
let DemangleOperator (nm: string) =
if nm.StartsWithOrdinal("( ") && nm.EndsWithOrdinal(" )") then
nm.[2..nm.Length - 3]
else
nm

type SuggestionBufferEnumerator(tail: int, data: KeyValuePair<float,string> []) =
let mutable current = data.Length
interface IEnumerator<string> with
member __.Current
with get () =
let kvpr = &data.[current]
kvpr.Value
interface System.Collections.IEnumerator with
member __.Current with get () = box data.[current].Value
member __.MoveNext() =
current <- current - 1
current > tail || (current = tail && data.[current] <> Unchecked.defaultof<_>)
member __.Reset () = current <- data.Length
interface System.IDisposable with
member __.Dispose () = ()

type SuggestionBuffer(idText: string) =
let data = Array.zeroCreate<KeyValuePair<float,string>>(maxSuggestions)
let mutable tail = maxSuggestions - 1
let uppercaseText = idText.ToUpperInvariant()
let allSuggestions = suggestionF()
let dotIdText = "." + idText
let mutable disableSuggestions = idText.Length < minStringLengthForSuggestion

let demangle (nm:string) =
if nm.StartsWithOrdinal("( ") && nm.EndsWithOrdinal(" )") then
let cleanName = nm.[2..nm.Length - 3]
cleanName
else nm
let insert (k,v) =
let mutable pos = tail
while pos < maxSuggestions && (let kv = &data.[pos] in kv.Key < k) do
pos <- pos + 1

/// Returns `true` if given string is an operator display name, e.g. ( |>> )
let IsOperatorName (name: string) =
if isNull name || not (name.StartsWithOrdinal("( ") && name.EndsWithOrdinal(" )")) then
false
else
let mutable i = 2
let mutable isOperator = true
while isOperator && i < name.Length - 3 do
if name.[i] = ' ' then
isOperator <- false
i <- i + 1
isOperator
if pos > 0 then
if pos >= maxSuggestions || (let kv = &data.[pos] in k <> kv.Key || v <> kv.Value) then
if tail < pos - 1 then
for i = tail to pos - 2 do
data.[i] <- data.[i + 1]
data.[pos - 1] <- KeyValuePair(k,v)
if tail > 0 then tail <- tail - 1

if allSuggestions.Contains idText then [] else // some other parsing error occurred
let dotIdText = "." + idText
allSuggestions
|> Seq.choose (fun suggestion ->
// Because beginning a name with _ is used both to indicate an unused
// value as well as to formally squelch the associated compiler
// error/warning (FS1182), we remove such names from the suggestions,
// both to prevent accidental usages as well as to encourage good taste
if IsOperatorName suggestion || suggestion.StartsWithOrdinal("_") then None else
let suggestion:string = demangle suggestion
let suggestedText = suggestion.ToUpperInvariant()
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
if similarity >= highConfidenceThreshold || suggestion.EndsWithOrdinal(dotIdText) then
Some(similarity, suggestion)
elif similarity < minThresholdForSuggestions && suggestedText.Length > minStringLengthForThreshold then
None
elif IsInEditDistanceProximity uppercaseText suggestedText then
Some(similarity, suggestion)
else
None)
|> Seq.sortByDescending fst
|> Seq.mapi (fun i x -> i, x)
|> Seq.takeWhile (fun (i, _) -> i < maxSuggestions)
|> Seq.map snd
|> Seq.toList
member __.Add (suggestion: string) =
if not disableSuggestions then
if suggestion = idText then // some other parse error happened
disableSuggestions <- true

// Because beginning a name with _ is used both to indicate an unused
// value as well as to formally squelch the associated compiler
// error/warning (FS1182), we remove such names from the suggestions,
// both to prevent accidental usages as well as to encourage good taste
if suggestion.Length >= minStringLengthForSuggestion && not (suggestion.StartsWithOrdinal "_") then
let suggestion:string = DemangleOperator suggestion
let suggestedText = suggestion.ToUpperInvariant()
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
if similarity >= highConfidenceThreshold ||
suggestion.EndsWithOrdinal dotIdText ||
(similarity >= minThresholdForSuggestions && IsInEditDistanceProximity uppercaseText suggestedText)
then
insert(similarity, suggestion) |> ignore

member __.Disabled with get () = disableSuggestions

member __.IsEmpty with get () = disableSuggestions || (tail = maxSuggestions - 1)

/// Formats the given predictions according to the error style.
let FormatPredictions normalizeF (predictions: (float * string) list) =
match predictions with
| [] -> System.String.Empty
| _ ->
let suggestions =
predictions
|> List.map (snd >> normalizeF)
|> List.map (sprintf "%s %s" System.Environment.NewLine)
|> String.concat ""
interface IEnumerable<string> with
member this.GetEnumerator () =
if this.IsEmpty then
Seq.empty.GetEnumerator()
else
new SuggestionBufferEnumerator(tail, data) :> IEnumerator<string>

" " + FSComp.SR.undefinedNameSuggestionsIntro() + suggestions
interface IEnumerable with
member this.GetEnumerator () =
if this.IsEmpty then
Seq.empty.GetEnumerator() :> IEnumerator
else
new SuggestionBufferEnumerator(tail, data) :> IEnumerator
Loading