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

Better method call error messages #71827

Closed

Commits on Dec 31, 2020

  1. Initial implementation for 65853

    This attempts to bring better error messages to invalid method calls, by applying some heuristics to identify common mistakes.
    
    The algorithm is inspired by Levenshtein distance and longest common sub-sequence.   In essence, we treat the types of the function, and the types of the arguments you provided as two "words" and compute the edits to get from one to the other.
    
    We then modify that algorithm to detect 4 cases:
    
     - A function input is missing
     - An extra argument was provided
     - The type of an argument is straight up invalid
     - Two arguments have been swapped
     - A subset of the arguments have been shuffled
    
    (We detect the last two as separate cases so that we can detect two swaps, instead of 4 parameters permuted.)
    
    It helps to understand this argument by paying special attention to terminology: "inputs" refers to the inputs being *expected* by the function, and "arguments" refers to what has been provided at the call site.
    
    The basic sketch of the algorithm is as follows:
    
     - Construct a boolean grid, with a row for each argument, and a column for each input.  The cell [i, j] is true if the i'th argument could satisfy the j'th input.
     - If we find an argument that could satisfy no inputs, provided for an input that can't be satisfied by any other argument, we consider this an "invalid type".
     - Extra arguments are those that can't satisfy any input, provided for an input that *could* be satisfied by another argument.
     - Missing inputs are inputs that can't be satisfied by any argument, where the provided argument could satisfy another input
     - Swapped / Permuted arguments are identified with a cycle detection algorithm.
    
    As each issue is found, we remove the relevant inputs / arguments and check for more issues.  If we find no issues, we match up any "valid" arguments, and start again.
    
    Note that there's a lot of extra complexity:
     - We try to stay efficient on the happy path, only computing the diagonal until we find a problem, and then filling in the rest of the matrix.
     - Closure arguments are wrapped in a tuple and need to be unwrapped
     - We need to resolve closure types after the rest, to allow the most specific type constraints
     - We need to handle imported C functions that might be variadic in their inputs.
    
    I tried to document a lot of this in comments in the code and keep the naming clear.
    Quantumplation committed Dec 31, 2020
    Configuration menu
    Copy the full SHA
    acd679c View commit details
    Browse the repository at this point in the history
  2. Adds some basic test cases

    Much more to come as code review progresses.
    Quantumplation committed Dec 31, 2020
    Configuration menu
    Copy the full SHA
    c266c4c View commit details
    Browse the repository at this point in the history
  3. Rust cleanup

    In a lot of these cases, I prefer the way I formatted the code, but I'll defer to the tidy script :)
    Quantumplation committed Dec 31, 2020
    Configuration menu
    Copy the full SHA
    7fc8e3c View commit details
    Browse the repository at this point in the history
  4. Prevent an ICE on empty params

    I still need to handle this properly, but I just threw this in to avoid the ICE.  I expect I'll have several rounds with code reviewers to refine things.
    Quantumplation committed Dec 31, 2020
    Configuration menu
    Copy the full SHA
    5bf45b3 View commit details
    Browse the repository at this point in the history

Commits on Jan 13, 2021

  1. Interpolate correct types into error messages

    For some reason I missed that you could just format!(ty) and get a human
    readable type for it.
    
    There's still lots to work out
     - What heuristics should we have around when to display the complicated
       labels? ex: with just one error, it's probably better for the user to
       provide the error but not the suggestion?  Are "missing" parameter
       suggestions, where we don't have something to stick in there useful?
     - The semantics around the spans to use; when should we eat a comma,
       dealing with multiple inserted params, etc.
    Quantumplation committed Jan 13, 2021
    Configuration menu
    Copy the full SHA
    f1916fa View commit details
    Browse the repository at this point in the history
  2. Fix a comment typo

    Co-authored-by: Joshua Nelson <joshua@yottadb.com>
    Quantumplation and Joshua Nelson committed Jan 13, 2021
    Configuration menu
    Copy the full SHA
    ae3c859 View commit details
    Browse the repository at this point in the history

Commits on Jan 14, 2021

  1. Small tweaks / typos fixed from code review

    No substantial changes here, just using more idiomatic rust and fixing
    typos in the comments
    Quantumplation committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    93ee0af View commit details
    Browse the repository at this point in the history
  2. Add a ton of test cases, refine error messages

    Refined the error messages a bit to be more context sensitive,
    added quite a few test cases.
    
    I'll need help adding cases with trait constraints and other tricky
    corner cases.
    Quantumplation committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    f369af6 View commit details
    Browse the repository at this point in the history

Commits on Jan 15, 2021

  1. Refactor final_arg_types to use a presized array

    Previously, it was a tuple including the index of the argument it was
    discussing, but this made it awkward to look up the types later.  The
    primary purpose of this was to pass it to another helper function, so I
    refactored that as well.
    
    Rather than trying to figure out the iterator chain that would work with
    the new structure, I just wrote the loop manually.
    Quantumplation committed Jan 15, 2021
    Configuration menu
    Copy the full SHA
    74faf96 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    d455267 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    d570d59 View commit details
    Browse the repository at this point in the history
  4. Fixed a bug related to invalid arguments

    Consider this case:
    
    ```
    fn some_func(a: i32, b: i32) {}
    some_func(1, "");
    ```
    
    Before this change, the first 1 would register as able to satisfy
    the second parameter, so the "" would show up as "extra" and we'd remove
    it.
    
    We would then allow the first argument to satisfy the first parameter,
    and the second parameter would have no argument, so it would show up as
    "missing".
    
    A simpler way to express this is just that the type of the second
    argument is invalid.  By doing one pass of satisfying arguments first,
    we can handle this case.
    Quantumplation committed Jan 15, 2021
    Configuration menu
    Copy the full SHA
    6eb1da0 View commit details
    Browse the repository at this point in the history
  5. Corrects the spans and types for the Invalid case

    Since the Invalid case covers a specific argument, we don't have to do
    span math or add commas, since all the comma, newline, etc. cases are in
    the original code
    Quantumplation committed Jan 15, 2021
    Configuration menu
    Copy the full SHA
    ddb50d3 View commit details
    Browse the repository at this point in the history
  6. A fix for a &str type check, and a pass at extra span alignment

    There were some test cases where "" wasn't typechecking against &str in
    the "soft" typechecking path (the one that doesn't persist new
    constraints.)
    
    I found a method that appears to typecheck without persisting new
    constraints, but I can't be sure, so I'll need to double check that.
    For now, this new expression typechecking method works well.
    
    This also tries to carefully handle eliminating extra arguments.  Spans
    are tricky, as we need to eliminate commas correctly, and the suggestion
    code gets angry if we have overlapping spans, so this tries to find
    contiguous blocks to eliminate.
    
    I'll probably simplify this now that I understand the logic a bit
    better, but it's late, so I'm going to bed.
    Quantumplation committed Jan 15, 2021
    Configuration menu
    Copy the full SHA
    b8c651f View commit details
    Browse the repository at this point in the history

Commits on Jan 16, 2021

  1. Add a multipart_suggestion_verbose

    This forces it to always show the subwindow for the suggestion.  Also uses HasPlaceholders when appropriate.
    Pi Lanningham committed Jan 16, 2021
    Configuration menu
    Copy the full SHA
    28b5a1f View commit details
    Browse the repository at this point in the history

Commits on Jan 17, 2021

  1. Give preference to latter commas

    Pi Lanningham committed Jan 17, 2021
    Configuration menu
    Copy the full SHA
    552583c View commit details
    Browse the repository at this point in the history
  2. Another round of improvements on spans

    The 'extra' spans are one iteration better, but still wrong, especially when there are arguments missing in the middle; Need another pass on this
    Pi Lanningham committed Jan 17, 2021
    Configuration menu
    Copy the full SHA
    de1a6c8 View commit details
    Browse the repository at this point in the history

Commits on Jan 19, 2021

  1. Refactor the check code to simplify the suggestions.

    Rather than dropping individual suggestion spans, I just re-construct
    the function call from scratch.  This makes fiddling with commas sooo
    much easier.
    
    The highlighting of individual errors is temporarily disabled, I'll add
    that back in temporarily.
    Pi Lanningham committed Jan 19, 2021
    Configuration menu
    Copy the full SHA
    807f656 View commit details
    Browse the repository at this point in the history

Commits on Feb 6, 2021

  1. Add back error span labels

    @estebank suggested we only annotate these if there are 5 or less.
    
    In addition to the suggestion at the bottom of the error, we label the
    specific problems we encounter without suggesting how to fix it.
    
    This also ladders the suggestion text: if there's just one thing to
    suggest, use specific language about the suggestion; however, if there
    are multiple suggestions, use a generic "did you mean"
    
    Even though the comment on span_suggestion suggests against this,
    Camelid on zulip pointed out that there are already a few suggestions
    like this, and any other language sounds slightly awkward.
    Quantumplation committed Feb 6, 2021
    Configuration menu
    Copy the full SHA
    c3dea90 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    1d6bab0 View commit details
    Browse the repository at this point in the history

Commits on Feb 7, 2021

  1. --bless error messages

    I left several tests failing, as they are likely cases my code needs to handle
    Quantumplation committed Feb 7, 2021
    Configuration menu
    Copy the full SHA
    5267285 View commit details
    Browse the repository at this point in the history