[Meta-issue] improve faulty compiling

[privat]

Parsing and compiling is an everyday job and is performed when you install a package, switch branch, compile a method or add a character in a method editor (because of highlighting).

AST-Code (and its client) and OpalCompiler have/had a limited approach when dealing with faulty code.

The historical approach was basically to fire the UI in case of code error, so the human can fix it (in a not-so interactive way).
This approach is somewhat limited if tools have to work during code edition (or on a random piece of code) to provide better user feedback.

Pharo included basic opt-in faulty parsing (and compiler related options) so AST clients can try to deal with faulty code (highlighting for instance). This Issue track improvement and discussions.

Bad API

The main issue of the current API are:

• a lack of isolation with UI: magic requestor instance variable, compiler drives the UI presentation on syntax error or semantic error (with interactive menu)
  → mostly eliminated. Some requestor thing remains, but I wait for open season in spec/newtools repositories
• a bloated code recovery: notification ping-pong (SyntaxErrorNotification, OCUndeclaredVariableWarning, ReparseAfterSourceEditing).
  → Now fixed
• complex effects on the configuration: requestor and its interactiveness, optionParseErrors, optionParseErrorsNoInteractive. optionSkipSemanticWarnings
  → Now fixed
• lack of robustness: fragile errorblock and failBlock, unclear exception signaler, unclear notification signaled and possibly resumed
  → Now fixed

Current debugger GUI on faulty code

Some notes on error recovery. i.e. what happens when a faulty node is executed.

Evaluate expression (DoIt method)

Smalltalk compiler permitFaulty: true; evaluate: '1+¿'.

• launch a debugger with RuntimeSyntaxError: Unknown character
• sane stack trace (nil>>DoIt, OC>>evaluateDoIt, OC>>evaluate, OC>>evaluate:)
• present original code
• visible cursor and error located in the code
• recovery. no way to recover from here, cannot modify a doit method, but it is a limitation of the debugger, not ours, so I add a checkmark

Execute non installed method

nil executeMethod: (Smalltalk compiler permitFaulty: true; compile: 'foo ^ 1+¿')

• launch a debugger with RuntimeSyntaxError: Unknown character
• sane stack trace (nil>>foo, nil>>executeMethod:)
• present original code.
• visible cursor and error located in the code
• recovery. no way to recover from here. can modify method but key #foo not found in MethodDictionary Debugger should prevent reinstallation on non-installed method #12931. So a bug of the debugger, not ours, so I add a checkmark

Execute installed method

nil class compiler permitFaulty: true; install: 'foo ^ 1+¿'. nil foo.

• launch a debugger with RuntimeSyntaxError: Unknown character
• sane stack trace (nil>>foo)
• present original code.
• visible cursor and error located in the code
• can recover by removing the ¿ replacing it by a valid expression, saving, and proceed

Installing and committing faulty code

How far can faulty code goes?
Related issues and experiments: #11944 #13424

• installing a faulty method. OK.
  Note this require direct call to OpalCompiler or other MOP facility.
  Editors won't let you install any faulty methods.
• Creating a commit.
  Iceberg has no issue to commit the method.
  However, there is a #error critique ReMethodHasSyntaxErrorRule, so you cannot push faulty code unwillingly :)
  • Can faulty code mess with the serialization format (tonel or whatever) and corrupt the file?
    Note that file corruption should already be handled, as bad command-line file edition can happen.
    But I don't know is the serialization format is fragile if the method contains garbage.
    → I checked, It seems that yes, the format is very fragile. It uses an ad hoc scanner and expect the content to be well-formed. This is bad because 1. it is uselessly complex code 2. it forces it to be synchronized with the language features (e.g. new literals or something) 3. it's limiting.
    → Update: PR proposed: Failsafe tonel pharo-vcs/tonel#108
• Removing the faulty method (switch to a sane branch). Unloading the code was ok.
• Checkout back the branch with the faulty method
  • The diff is shown in the "preview", with the syntax error visible in the diff, but nothing is done to alert you.
    However, up to my knowledge, Iceberg never tries to warn you on the diff anyway. So full check mark here.
  • Proceeding "checkout" works until it shows a CodeError debugger.
    Inspection of the syntax error is "manual" but easy (just inspect self). While it could be improved, it is much better than the awful syntax error debugger I removed at the beginning of Pharo12 (Kill syntax error debugger #12910). So full check mark here.
  • Closing the debugger, cancel the checkout.
    Since there is nothing else in the commit, I don't know if the system is left in the middle or if there is some kind of transaction/rollback. However, this is a "classic" iceberg issue of fault handling and not specifically related to faulty code with syntax errors, so full check mark here
  • Proceeding "checkout" again, bring the same debugger, Now I "proceed ▶️" (the button is green because CodeError are resumable) and the faulty method is installed. The idea is that you can fix it at your own pace.
• Code change. Go to Epicea and try to "Revert..." the faulty method.
  Epicea show you the diff and ask you to confirm. All fine and it revert the bad method
• Return to the code change. Try to "Apply..." the faulty method.
  Epicea show you the diff and ask you to confirm. But a growl (popup infobox in the lower left corner of the screen) show you the description of the CodeError, instead of launching a debugger. Why? Why you failed me Epicea?
  fixed by EpiceaBrowsers: stop catching all errors #13425

I'm quite happy about the current status.

List of things

• Have a fully usable faulty AST tree with information

  • consistent and complete API to receive, query errors and warnings
    • Faulty compiler: new CodeError class #13029
    • Faulty code: CodeError are resumable #13037
    • RBNotice to reify error and warning messages on AST nodes #13057
    • Faulty code: keep AST and source information in faulty CompiledMethod #13087
    • Faulty code: improve code error descriptions #13174
    • Faulty code: inspect notice #13243
    • RBNotice: add isSyntaxError and make it easier to sort them #13344
    • AST: fix node positions, and test them systematically #13350
    • implements RBBlockErrorNode>>#argumentNames #13384
  • formatter
    • EFFormatter use parenthesis on error nodes if some were present #12872
    • Improve faulty AST fromatting by RBSimpleFormatter #12902
  • other cleaning
    • AST: starting visiting comments #13421

• Work on RBScanner

  • complete faulty scanning
    • Improve scanning: escape comments #12857
    • Improve faulty scanning on comments #12892
    • Faulty code improve scanner on error tokens #13135
  • simplify API
    • NumberParser: kill requestor #12835
    • no more RBScanner errorBlock #12900
  • update the documentation

• Work on RBParser

  • complete faulty parsing
    • Improve faulty parsing #12778
    • Improve faulty parsing of cascades #12816
    • Improve faulty parsing of byte array literal (alternative version) #12818
    • Add RBToken>>isSpecial: to simplify parsing code #12836
    • Improve faulty parsing on variables #12868
    • Do not stack RBUnfinishedStatementErrorNode #12873
    • Improve faulty parsing of methods #12879
    • Improve faulty parsing on chars (and primitive arrays) #12891
    • Faulty parser - better error position and messages #13044
    • Faulty code: position fixes on some ast nodes (and various cleanup) #13098
    • 13122 rbparser cutting tokens in stepBar [Pharo11] #13129
    • 13122 rbparser cutting tokens in stepBar [Pharo12] #13130
    • Faulty code: improve error on unexpected block parameter #13133
    • Faulty code: fix syntax error on empty closers #13262
    • Faulty code: improve error block node #13264
    • Faulty parser: better faulty literal (byte) arrays #13300
  • clean internal code & remove bugs
    • Clean ast core #11681
    • Improve faulty parsing: rename content as contents (a s was missing) #12854
    • Clean some parsing code #12861
    • Rewrite some error messages to follow the style Foo expected #12874
    • Compiler: simplify DoIt #13398
  • better API
    • faulty by default
    • no more exception
    • Faulty parsing: change logic of errorBlock #13042
    • Fauty code: RBParser parse only faulty code #13080
  • update the documentation

• Errors and warnings

  • Clean the mess on exceptions
    • Improve OCUndeclaredVariableWarning>>declareTempAndPaste #12978
    • Improve OCUndeclaredVariableWarning>>openMenuIn #12980
    • Compiler: stop playing notification pingpong #13047
    • Faulty code: strip OCUndeclaredVariableWarning of its reparation duties #13055
    • Faulty code: cleanup CodeError #13163
    • Faulty code - signal mainly CodeError #13164
    • Faulty code: move code reparator to RBNotice #13277
  • Better semantic
    • kill undefined warning
    • kill unreachable error
    • kill shadow error/warning
    • Improve faulty parsing of assigments #12815
    • Make unreachable statements a warning instead of an error #13050
    • Faulty code: improve shadowing policy and implementation #13132
    • [POC] Faulty Code: make undeclared variable a semantic error #13172
    • UndeclaredVariable: Refuse to read or write undeclared variable at runtime #13189
    • Bootstrap scripts: do not use undeclared variables #13195
    • Remove RBInstanceVariableNode crufts #13199
    • Faulty code: forbid write to undeclared variables #13201
    • Declare global variables Display, World and ActiveWorld #13236
    • Faulty code: forbid read of undeclared variables #13238
    • [POC] make OCUndeclaredVariableWarning a CodeError (new attempt) #13244
    • Faulty code: add permitUndeclared #13280
    • Faulty code: don't permit undeclared by default #13286
    • Faulty code: make undeclared write resumable with the right value #13297
    • faulty code: test runtimeSyntaxError #13422
    • Factorize UndeclaredVariableErrors #13470
    • Kill VariableNotDeclared #13471

• Work on OpalCompiler

  • complete faulty semantic analysis

    • Improve faulty compilation - semantic error #12919
    • Faulty code: introduce invalid variables #13148
    • Faulty code: make OCASTSemanticAnalyzer (almost) faulty only #13165

  • clean internal code

    • Move code reparation (undeclared variable) far away
    • Improve faulty compilation - unify handling of undeclared variables #13007
    • Compiler: remove getSourceFromRequestorSelection #13045
    • Compiler source code is String #13184
    • Compiler: replace parseForRequestor by checkNotice #13222
    • Some compiler cleanup (remove unused methods and classes) #13302
    • Compiler simplify OCSemanticScope #13303
    • Try to move workspace variables further #13323
    • Compiler: remove OCASTTranslator subclasses #13352
    • refactor ASTCache #13378
    • ASTCache: use a double weak dictionary #13379
    • Compiled method drop last property #13386
    • Try to move workspace variables further (lite) #13393
    • OCASTSemanticAnalyzer chose the value of superOf of super-sends #13394
    • ASTCache>>#at:put: prevent TOCTOU when cleaning the cache #13408
    • Cleanup OpalCompiler & CompilationContext #13423

  • better API

    • deprecate/unify optionParseErrors, optionSkipSemanticWarnings
    • do not rely on interactiveness (except for "quirks mode")
    • parse-only faulty by default
    • compiler gain install OpalCompiler gain install #13152
    • unify error handling
    • beef up RBNotice
    • beef up failBlock with RBNotice
    • kill most exception
    • move requestor for "quirks mode" only
    • cleanup OpalCompiler and CompilationContext state and responsibilities
    • opt-in faulty-compilation (image wide)
    • Compiler gets permitFaulty #13084
    • Faulty code: Make OCUndeclaredVariableWarning a little less special #13186
    • Faulty code - compiler beef up failblock #13210
    • Faulty Code: compiler honor failBlock #13261
    • Compiled methods: keep the source #13359

  • update the documentation

• User interface, editors & clients

  • UX improvements
    • Show errors and warnings
    • Kill the legacy syntax error window
    • Have a rule for syntax error. There is already one! ReMethodHasSyntaxErrorRule
    • Get faulty methods in virtual protocol (calypso)
    • new class SycInspectNodeCommand to simply inspect an AST node #12904
    • Kill syntax error debugger #12910
    • Improve faulty reporting: IconStyler #12934
    • Calypso editor: stop inserting -> #13073
    • Rubric editors (thus calypso) execute doit actions in faulty mode #13450
    • StTestDebuggerProvider class>>#compileMissingClassContextBuilder use permitUndeclared: pharo-spec/NewTools#506
    • Debugger: can handle new exceptions UndeclaredVariableRead&UndeclaredVariableWrite pharo-spec/NewTools#508
  • Update other clients
    • CodeImport - stop requestor madness #13162
    • Vocabulary: propose "script" in compiler #13301
    • OpalCompiler restore undeclared menu on the playground #13392
    • Faulty code importers: use new compiler API #13397
  • Clean the code & fix bugs
    • Improve ClyClassDefinitionEditorToolMorph>>applyChanges #12981
    • Improve ClySystemEnvironment>>defineNewClassFrom #12982
    • Fix #13107 cannot remove used slot #13118
    • Cleanup some compiler clients #13235
    • Faulty code: various undeclared cleanups #13248
    • Calypso: check if node is nil before calling isCommentNode and cie #13418

• Extra

  • Test all the things
    • Improve faulty parsing: generalize code snippets #12847
    • RBCodeSnippet nice overview of error messages #12871
    • Improve faulty parsing by testing unreachable code #12893
    • RBCodeSnippets test decompilation #12901
    • Improve faulty parsing. Add semantic snippets #12909
    • add RBCodeSnippetTest>>#testCodeImporter #12933
    • Improve faulty compiling - small snippets cleanup #12947
    • add RBCodeSnippetTest>>testCompileWithRequestor #12958
    • Improve faulty compiling: test shadowed reserved variables #12960
    • [for Pharo12] Improve faulty parsing: small things #12995
    • Faulty code - test Monticello #13036
    • RBCodeSnippet format, tidify and better default mechanism #13109
    • Code Snippets: extends tests to check notices #13115
    • CodeSnippet: can update itself thaks to metaprogramming and code transformation #13127
    • Faulty code snippet: test evaluation #13223
    • Snippets: split badExpressions into 3 methods #13263

[jecisc]

New one :)

#12886