Enhanced errors #874

Thizizmyname · 2018-11-26T20:07:39Z

This pull request overhauls the visual style of encores error handling with the following functionalities:

Use of ANSI escape sequences to colorize error and warning output in a structured format.
Enables the ability to provide a hash value based on the type of error given, this can request further information of how and why these kinds of errors occur on a more general basis.
Enables the ability for the encore compiler to analyze the environment when an error has occurred and come up with suggestions on how to correct the fault.

Error output and structure

The ANSI escape sequences are removed it it does not detect a terminal as output, meaning if the output is redirected to a file or to a pipe, it will remove the color sequences and so that the output will remain clean.

Error explanations

If an error occurs that have an explanation it will display a hash-value in which you can provide to the compiler for further information.

$ encorec --explain E002

This will open up a pager with the text explaining a general case where this kind of error can occur.

Code analyzing

In the image we can see an inlined notation by the highlighted error and a longer note underneath. These notation can contain small information on what it expected and what you might wanted to do.
Suggestions available and small list of potential suggestions:

expected / found types upon type mismatch.
prints method if wrong number of arguments is provided.
Suggest function if a non-existing method is called but a function with same made is provided.
Exposing a private method if used outside its module (not implemented).
Potential candidates to a function- / method-call with a typographical error (not implemented).

Limitations / TODOs

Range error

Due to the way the parser consumes whitespaces before determine end of token, there is a visual bug were the range of each token is the length of the token plus all spaces until the next token.

Error: Left-hand side of operand is not assignable
   --> "plusplus.enc" (Line:35, Column:19)
   |
35 |         (e*2) +=    3)
   |               ^^^^^^ Can only be used on var or fields

Inefficient Suggest analyzation

The result for the inlined suggestion as well as the longer underneath are handled by two separate functions, making it not possible to take advantage of the ability of determining after the fact what kind of message should be displayed where, if these were produced as one function it would be more efficient to not needing to re-evaluate values as well as determining the position based on the size or complexity of the result.

- Only shows the new error on NonAssignableLHSError - Only displays the first line of the errored code, if the expression/etc is segmented on multiple lines it will for now not show the entire part

purification

Extracted the printer to own file so that TypeError only holds the data structures and text for different error descriptions/suggestions. Also removed all deprecated code.

This is due to the visual bugs in prettyprinter.hs introduced by the differences on how the libraries handle empty documents as well as differences with functions such as nest and hang.

Need to expose the annotated version of Doc in prettyprinter to apply prettyprinted text in suggestions

warnings

kikofernandez

I really like this PR. The code is quite understandable and reading the error messages is much much better than what we had in place before. I have added refactorings, suggestions and spotted a few places that I think need a bit of a clarification, before we merge this.

One thing that I do not know if it is too difficult, would be to basically compile an Encore program with the explanation flag and, if there are any errors, show not only the place and backtrace of the error, but also the suggestion that matches the appropriate code. This way, if I get an error, I do not have to chase the error / warning code to get the good explanation.

As I said, really good PR. Looking forward to the changes :)

src/types/Typechecker/ExplainTable.hs

kikofernandez · 2019-02-06T12:28:37Z

src/types/Typechecker/Errorprinter.hs

+showPosition :: Position -> Doc TCStyle
+showPosition pos = styleLogistic (text "-->") <+> (text $ show $ pos)
+
+codeViewer_ver1 :: (TCType a, Suggestable a) => a -> [String] -> Doc TCStyle


which of the codeViewers is the right one?

kikofernandez · 2019-02-06T12:34:41Z

src/types/Typechecker/Errorprinter.hs

+
+pipe = char '|'
+
+declareError :: Error -> Doc TCStyle


The declareError and declareWarning are pretty much the same except for the hash format and the word "Error" or "Warning". This fact make it good for a refactoring. I would recommend:

Create in ExplainTable.hs the following class and instances and remove the methods lookupHash and lookupHashW:
class Explanainable a where explain :: a -> Maybe Int instance Explanainable Error where explain err = toKey err instance Explanainable Warning where explain warn = toKeyW warn

This basically allows you to have ad-hoc polymorphism over the types, which is what you want. We want to be able to call the method explain without having to call one specific method for errors and another one for warnings.

Update the functions declareError and declareWarning to the following ones:

declareError :: Error -> Doc TCStyle declareError = printText "[E%04d]" "Error" . explain declareWarning :: Warning -> Doc TCStyle declareWarning = printText "[W%04d]" "Warning" . explain printText _ _ Nothing = empty printText format msg (Just num) = let hash = text $ printf format num in styleClassify $ text msg <> hash <> char ':'

In my personal opinion, this is more understandable than the function before and it hides internal facts that are not needed for the functions declareWarning and declareError, such as that there is an option type.

I like it! it has been changed.

This solution would not produce the correct result. it need to do:
styleClassify $ text msg ______ <> char ':'
Regardless of the Maybe-type. And I like to preserve not having two lines that has to be the same. I have reformatted the function a little hoping to convey that more properly.

kikofernandez · 2019-02-06T12:37:08Z

src/front/TopLevel.hs

@@ -14,6 +14,9 @@ import System.Directory
 import System.IO
 import System.Exit
 import System.Process
+import System.Posix.Process


would it be possible to import only the things that you are using?
Unless you really require a lot of functions from the modules imported here.

Basically, I do not know what combinator from System.Posix.Process, System.Pager, System.Console.ANSI you are using.

src/front/TopLevel.hs

kikofernandez · 2019-02-06T12:59:28Z

src/types/Typechecker/Suggestable.hs

+makeNotation :: Doc TCStyle
+makeNotation = styleLogistic (pipe $+$ equals) <+> styleDesc (text "note:")
+
+-- How to determine if to use a smallSuggest or longSuggest:


Since it seems that we can choose between small and long suggestions, can we choose one version from the version1 or version2 and change the small or long suggestions as we see fit? Maybe adding some documentation at the beginning of the module can clear things up.

I could definitely add some documentation.

What do you mean version1 or version2? the codeviewer?
There are two versions right now doing the same thing, one is ment to be removed and amended before merging.
It's made so that you could construct different codeviewers and choose based on the error
(an example is if you have the wrong arguments for a function, the code viewer could display the actual function declaration as well as where the programmer wrote wrong. I think the declaration s range also includes the entire body which is why I never made such a viewer)

The small and long suggest will always print, which is why default is empty (nothing will print).
A desire that I included as future work was to refactor the suggestions to be returned as one function, meaning that you could evaluate the code, and depending on how much information you want to relay back you could then choose if it would be displayed as a short or long suggest (or both). Right now you would need to compute this for both functions individually.

src/types/Typechecker/Typechecker.hs

kikofernandez · 2019-02-06T13:09:56Z

src/types/Typechecker/Errorprinter.hs

+
+
+printError :: TCError -> IO ()
+printError err@(TCError _ Env{bt = []}) =


the TCError ADT was updated from TCError Error Backtrace to TCError Error Env. However, I believe that you never make use of the environment when you try to print an error. I was wondering if you really need the environment in the TCError. I am sure I am wrong, but I could not find where do you make use of the Env from the TCError.

The Env is passed along to the long and short suggest. Otherwise we could not evaluate an error and give suggestions.

right! I just saw that you need the environment for a specific case of the long suggest. The short suggest doesn't seem to use it

short suggest uses it as well to figure out if the name of an unknown method is available as a function. (As I would like to refactor this code into a single function returning both the possible short and long suggestions it would be wise to leave the arguments the same)

kikofernandez · 2019-02-06T13:12:01Z

src/types/Typechecker/Suggestable.hs

+        | isMethodNameAFunction name ty env = text $ printf "Did you mean function `%s`?" (show name)
+    smallSuggest _ = empty
+
+    longSuggest (TCError (TypeMismatchError actual expected) _) =


Maybe the first two cases of longSuggest can be refactored using guards. They are an identical copy except from the error type/

As a TypeWithCapabilityMismatchError has a very "ugly" description I think it might be more wise to remove it entirely from suggestible as a way of subtly informing that this error needs rework.

kikofernandez · 2019-02-06T13:13:44Z

src/types/Typechecker/Suggestable.hs

+
+
+
+isMethodNameAFunction name ty env =


I would suggest to put this method inside a where clause in the place where it is used, since it does not seem to be a combinator that is needed for multiple functions.

I wanted these abstracted helper-functions to not be local so that it could be used by anything, specially as long as short and long-suggest are two different functions.

A desire that I included as future work was to refactor the suggestions to be returned as one function [...] Right now you would need to compute this for both functions individually.

kikofernandez · 2019-02-15T07:17:28Z

Before merging, I would like to see if the other comments are addressed (or there is a reply).
@Thizizmyname As soon as you address the remaining comments and fix the current conflicts, I can merge this. Thanks!

Thizizmyname and others added 30 commits May 25, 2018 13:43

Basic implementation of new error-printer

abd0f35

- Only shows the new error on NonAssignableLHSError - Only displays the first line of the errored code, if the expression/etc is segmented on multiple lines it will for now not show the entire part

test for NonAssignableLHSError

0a8274a

Elias TCError now takes Environment instead of Backtrace

ddc9b0b

Added Elias patch to include Environment into TCError

1bf6d72

Update output of tests

8f4f265

Updated Meta.hs for new way to print position

3131c66

Printer accepts multi-line errors with no additional arguments

3939c13

Optimized code-input as well as refactored for future function

f826e76

purification

Reverted back to not show ranges in error position

b08e8b5

Swapped Text.PrettyPrint to Data.Text.Prettyprint.Doc

ec8364a

Remade errorprinter to use rich text Doc instead of ANSI Console

a1a3acb

Refactoring and modularization

a63903d

Extracted the printer to own file so that TypeError only holds the data structures and text for different error descriptions/suggestions. Also removed all deprecated code.

Reverted to use pretty-1.1.3.* instead of prettyprinter-1.2.1

b793649

This is due to the visual bugs in prettyprinter.hs introduced by the differences on how the libraries handle empty documents as well as differences with functions such as nest and hang.

Added annotation for code blocks

6303390

List visible methods/functions and added method suggestions

8987ed4

Minor touchups

be98118

Added functions for method-name analysis and refactorization

48d95fd

Need to expose the annotated version of Doc in prettyprinter to apply prettyprinted text in suggestions

stupid typo

5452465

Working explainer with sub-par data structure

72dff6b

Added rudimentary fixed table

c710af5

Ability to construct explanations of different error types

b31315b

Re-added instance Show for Error

64297e4

Added TypeMismatchError and explanation

cbbea76

Expose default function modules in visibleFunctions

959717f

Test using external file and pager for explanations

3d9d3ff

Code cleanup

0d76dfe

Updated to latest 'development' branch

f1473cc

Alternative codeviewer with new multiline renderer

390d593

Added pretty rendering of warnings

6c07c05

Uses now pager for every explanation

02874a5

Thizizmyname added 2 commits November 26, 2018 02:02

Added functionality for external explanations for specific errors and

3905379

warnings

Added graphical tests suite

ddad6e8

kikofernandez requested changes Feb 6, 2019

View reviewed changes

Moved auxiliary info to long suggest

475b895

Thizizmyname mentioned this pull request Mar 17, 2019

Type inconsistency of default values in headers are not pre-checked #876

Closed

Thizizmyname and others added 4 commits March 27, 2019 20:11

Update from feedback

6f9b25b

Merge branch 'development' into enhanced-errors

ce3ee62

feedback resolved and workarounds to solve stupidity

53ef4a6

Updated from upstream parapluu/encore

dbdfa01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced errors #874

Enhanced errors #874

Thizizmyname commented Nov 26, 2018

kikofernandez left a comment

kikofernandez Feb 6, 2019

kikofernandez Feb 6, 2019

Thizizmyname Jan 30, 2020

kikofernandez Feb 6, 2019

kikofernandez Feb 6, 2019

Thizizmyname Feb 14, 2019

kikofernandez Feb 6, 2019

Thizizmyname Feb 14, 2019

kikofernandez Feb 15, 2019

Thizizmyname Mar 27, 2019

kikofernandez Feb 6, 2019

Thizizmyname Mar 27, 2019

kikofernandez Feb 6, 2019

Thizizmyname Feb 14, 2019

kikofernandez commented Feb 15, 2019



		printError :: TCError -> IO ()
		printError err@(TCError _ Env{bt = []}) =

Enhanced errors #874

Are you sure you want to change the base?

Enhanced errors #874

Conversation

Thizizmyname commented Nov 26, 2018

Error output and structure

Error explanations

Code analyzing

Limitations / TODOs

Range error

Inefficient Suggest analyzation

kikofernandez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kikofernandez commented Feb 15, 2019