Give readable error on lua validation failure

[GiovanH]

No description provided.

[dansanderson]

Closing this PR for now but still thinking about this. Thanks again!

[GiovanH]

I still find an edit like this essential for debugging lua errors or cases where picotool parses the code wrong. Are there any changes you would want made before merging something like this into the tool?

[dansanderson]

@GiovanH It's possible that the proposed change is complete enough and I'm just missing something, but at first glance this only catches and reports a small subset of errors, and only when using the LuaASTEchoWriter. I wanted to think more about where this kind of problem might occur, and how much of the system should be exposed to that information.

I'd also want to clean up the error reporting strategy so that it's not based on an assert in this way. Any exceptional condition that prevents progress but is known to occur in the wild should be reported as a specific exception, with useful info (offending symbol, line and character location) attached as an exception payload. As written, the assert is supposed to confirm a condition that can't be false unless there's a serious bug in picotool. This is actually trying to catch cases where picotool doesn't understand specific user input.

Also, LuaASTEchoWriter shouldn't be printing error messages. Even if this error can't practically be caught until processed by the writer, it should throw an exception that whatever tool using the Writer can catch and report to the user.

Separately, it sounds like we're looking to catch and report a distinction between "this user input is wrong" and "this user input is probably right but picotool can't handle it." I'm curious if there's a way to do that for known categories of gaps in picotool.

Let me know your thoughts!

[GiovanH]

Yeah, I'm encountering most of these issues chasing down other input bugs (#92) rather than trying to make a robust LUA code troubleshooter.

More recently I've simply modified the existing assert statement in lua.py itself to at least give some information about the tokens:

    def _get_text(self, node, keyword):
        """Gets the preceding spaces and code for a TokKeyword or TokSymbol.

        Args:
          node: The Node containing the keyword.
          keyword: The expected keyword or symbol.

        Returns:
          The text for the keyword or symbol.
        """
        if self._args.get('ignore_tokens'):
            return b' ' + keyword

        spaces = self._get_code_for_spaces(node)
        assert (self._tokens[self._pos].matches(lexer.TokKeyword(keyword)) or
                self._tokens[self._pos].matches(lexer.TokSymbol(keyword))), f"{self._tokens[self._pos]} != {lexer.TokSymbol(keyword)} ({keyword})"
        self._pos += 1
        return spaces + keyword

[dansanderson]

I'd take that as written, if that helps. The message is at least wrapped in an exception that way, and not printed directly by the Writer.

[GiovanH]

Just looked at my original pr, and yikes. Yeah, let me rewrite that.