Parser weak at diagnosing specification errors

[jeff5]

Although the hard work is done by the TOML parser itself, we are responsible for extracting a block of TOML. This is a fairly simple line-oriented process. The code that does it seems to parse correct metadata correctly, but is easily confused into silently ignoring the whole thing.

There is an example program in the PyPA spec with a regex, which is marginally better at validation than jython-cli, but being a regex, it also silently ignores the whole when anything is slightly wrong.

This is incorrect (according to PyPA) because it specifies jbang twice.

# /// jbang
# debug = true
# requires-jython = "2.7.2"
# requires-java = "8"
# ///

# /// jbang
# debug = true
# requires-jython = "2.7.3"
#
# [java]
#   runtime-options = ""
# ///

This is accepted as a jbang block, but shouldn't be:

# /// jbangbang
# debug = true
# requires-jython = "2.7.2"
# requires-java = "8"
# ///

This is silently ignored (no closing # ///):

# /// jbang
# debug = true
# requires-jython = "2.7.2"
# requires-java = "8"

And if there is a following valid jbang block, then that is ignored too. (The PyPA regex parses the second one.)

How do we feel about extra metadata that jython-cli doesn't understand? TOML has no opinion about what names are valid, and it can be a useful flexibility during innovation. But it is also what a spelling mistake looks like. I'm thinking a warning would be useful if there are items left over that we didn't use.

[jeff5]

Starter pack for exploration:

# pypa_check.py Reference scanning of metadata

# Based on:
# https://packaging.python.org/en/latest/specifications/inline-script-metadata/#inline-script-metadata

import sys
import re
import tomllib

from pprint import pprint

REGEX = r'(?m)^# /// (?P<type>[a-zA-Z0-9-]+)$\s(?P<content>(^#(| .*)$\s)+)^# ///$'

def read(script: str, name: str='script') -> dict | None:
    matches = list(
        filter(lambda m: m.group('type') == name, re.finditer(REGEX, script))
    )
    if len(matches) > 1:
        raise ValueError(f'Multiple {name} blocks found')
    elif len(matches) == 1:
        content = ''.join(
            line[2:] if line.startswith('# ') else line[1:]
            for line in matches[0].group('content').splitlines(keepends=True)
        )
        return tomllib.loads(content)
    else:
        return None


with open(sys.argv[1], 'rt') as f:
    text = ''.join(f.readlines())
    pprint(read(text, 'jbang'))

Ring the changes on this to explore different errors. It includes a valid examples from the standard, as these have to be passed over by our parser (and are at present).

# cruel2.py Torture jython-cli in various ways

# /// script
# requires-python = ">=3.11"
# dependencies = [
#   "requests<3",
#   "rich",
# ]
# ///

# /// some-toml
# embedded-csharp = """
# /// <summary>
# /// text
# ///
# /// </summary>
# public class MyClass { }
# """
# ///

# /// jbangbang
# debug = true
# requires-jython = "2.7.2"
# requires-java = "8"

# /// jbang
# debug = true
# requires-jython = "2.7.3"
# horseplay = true
#
# [java]
#   runtime-options = ""
# ///

import sys
from java.lang import System

print(sys.argv)
print("sys.stdout.encoding = " + sys.stdout.encoding)
print("sys.stdin.encoding = " + sys.stdin.encoding)

v = sys.version_info
print "Jython %d.%d.%d" % (v.major, v.minor, v.micro)
print "Java " + System.getProperty('java.version')

[wfouche]

Output produced JythonCli.java and script cruel2.py:

Windows

C:...> jbang run .\JythonCli.java cruel2.py
[jython-cli] [jbang.cmd, run, --java, 21, --main, org.python.util.jython, org.python:jython-slim:2.7.3, cruel2.py]
['cruel2.py']
sys.stdout.encoding = cp850
sys.stdin.encoding = cp850
Jython 2.7.3
Java 21.0.7

Linux

$ jbang run JythonCli.java cruel2.py 
[jython-cli] [jbang, run, --java, 21, --main, org.python.util.jython, org.python:jython-slim:2.7.3, cruel2.py]
[jbang] Resolving dependencies...
[jbang]    org.python:jython-slim:2.7.3
[jbang] Dependencies resolved
['cruel2.py']
sys.stdout.encoding = UTF-8
sys.stdin.encoding = UTF-8
Jython 2.7.3
Java 21.0.7

[wfouche]

JythonCli.java updated to ignore all TOML data if more than one valid /// jbang content block is detected.

# /// jbang
# debug = true
# requires-jython = "2.7.3"
# horseplay = true
#
# [java]
#   runtime-options = ""
# ///

# /// jbang
# debug = true
# requires-jython = "2.7.1"
# horseplay = true
#
# [java]
#   runtime-options = ""
# ///

import sys
from java.lang import System

print(sys.argv)
print("sys.stdout.encoding = " + sys.stdout.encoding)
print("sys.stdin.encoding = " + sys.stdin.encoding)

v = sys.version_info
print "Jython %d.%d.%d" % (v.major, v.minor, v.micro)
print "Java " + System.getProperty('java.version')

In this example, it ignores the two conflicting jbang specifications, and defaults to Jython 2.7.4.

jbang run JythonCli.java cruel2.py 
[jython-cli] error - multiple jbang content blocks detected
['cruel2.py']
sys.stdout.encoding = UTF-8
sys.stdin.encoding = UTF-8
Jython 2.7.4
Java 21.0.7

[wfouche]

@jeff5 , the changes are ready to be reviewed.

[jeff5]

What if I just forget the end marker, or place a properly blank line in the block, thinking that's ok?

[wfouche]

What if I just forget the end marker, or place a properly blank line in the block, thinking that's ok?

In both cases the jbang content block is malformed and is ignored.

Tested these two conditions with

# /// jbang
# debug = true
# requires-jython = "2.7.2"
# horseplay = true
# [java]
#   runtime-options = ""

import sys
from java.lang import System

print(sys.argv)
print("sys.stdout.encoding = " + sys.stdout.encoding)
print("sys.stdin.encoding = " + sys.stdin.encoding)

v = sys.version_info
print "Jython %d.%d.%d" % (v.major, v.minor, v.micro)
print "Java " + System.getProperty('java.version')

and

# /// jbang
# debug = true
# requires-jython = "2.7.2"
# horseplay = true

# [java]
#   runtime-options = ""
# ///

import sys
from java.lang import System

print(sys.argv)
print("sys.stdout.encoding = " + sys.stdout.encoding)
print("sys.stdin.encoding = " + sys.stdin.encoding)

v = sys.version_info
print "Jython %d.%d.%d" % (v.major, v.minor, v.micro)
print "Java " + System.getProperty('java.version')

[Stewori]

In both cases the jbang content block is malformed and is ignored.

I think Jeff's point is that the malformed block should not be silently ignored. It would be nice to get a warning issued.

[wfouche]

An error recovery step has been implemented. The following malformed jbang content block

# /// jbang
# debug = false
# requires-jython = "2.7.2"
# horseplay = true

# [java]
#   runtime-options = ""
# ///

import sys
from java.lang import System

print(sys.argv)
print("sys.stdout.encoding = " + sys.stdout.encoding)
print("sys.stdin.encoding = " + sys.stdin.encoding)

v = sys.version_info
print "Jython %d.%d.%d" % (v.major, v.minor, v.micro)
print "Java " + System.getProperty('java.version')

is detected and displayed to the console

$ jbang run JythonCli.java cruel2.py
[jython-cli] error - malformed jbang content block discarded
# /// jbang
# debug = false
# requires-jython = "2.7.2"
# horseplay = true
['cruel2.py']
sys.stdout.encoding = cp850
sys.stdin.encoding = cp850
Jython 2.7.4
Java 21.0.7