Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking

[John Snow]

On 3/24/21 1:57 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > On 3/23/21 5:40 AM, Markus Armbruster wrote:
> > > Naming rules differ for the various kinds of names.  To prepare
> > > enforcing them, define functions to check them: check_name_upper(),
> > > check_name_lower(), and check_name_camel().  For now, these merely
> > > wrap around check_name_str(), but that will change shortly.  Replace
> > > the other uses of check_name_str() by appropriate uses of the
> > > wrappers.  No change in behavior just yet.
> > > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > > ---
> > >    scripts/qapi/expr.py | 51 +++++++++++++++++++++++++++++++-------------
> > >    1 file changed, 36 insertions(+), 15 deletions(-)
> > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> > > index e00467636c..30285fe334 100644
> > > --- a/scripts/qapi/expr.py
> > > +++ b/scripts/qapi/expr.py
> > > @@ -21,11 +21,12 @@
> > >    from .error import QAPISemError
> > >      
> > > -# Names must be letters, numbers, -, and _.  They must start with letter,
> > > -# except for downstream extensions which must start with __RFQDN_.
> > > -# Dots are only valid in the downstream extension prefix.
> > > -valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?'
> > > -                        '[a-zA-Z][a-zA-Z0-9_-]*$')
> > > +# Names consist of letters, digits, -, and _, starting with a letter.
> > > +# An experimental name is prefixed with x-.  A name of a downstream
> > > +# extension is prefixed with __RFQDN_.  The latter prefix goes first.
> > > +valid_name = re.compile(r'(__[a-z0-9.-]+_)?'
> > > +                        r'(x-)?'
> > > +                        r'([a-z][a-z0-9_-]*)$', re.IGNORECASE)
> > >      
> > >    def check_name_is_str(name, info, source):
> > > @@ -37,16 +38,38 @@ def check_name_str(name, info, source,
> > >                       permit_upper=False):
> > >        # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty'
> > >        # and 'q_obj_*' implicit type names.
> > > -    if not valid_name.match(name) or \
> > > -       c_name(name, False).startswith('q_'):
> > > +    match = valid_name.match(name)
> > > +    if not match or c_name(name, False).startswith('q_'):
> > >            raise QAPISemError(info, "%s has an invalid name" % source)
> > >        if not permit_upper and name.lower() != name:
> > >            raise QAPISemError(
> > >                info, "%s uses uppercase in name" % source)
> > > +    return match.group(3)
> > > +
> > > +
> > > +def check_name_upper(name, info, source):
> > > +    stem = check_name_str(name, info, source, permit_upper=True)
> > > +    # TODO reject '[a-z-]' in @stem
> > > +
> >
> > Creates (presumably) temporary errors in flake8 for the dead
> > assignment here and below.
>
> All gone by the end of the series.
>
> "make check" and checkpatch were content.  Anything else you'd like me
> to run?

Eventually it'll be part of CI, with targets to run locally.

I never expected the process to take this long, so I did not invest my 
time in developing an interim solution.

I use a hastily written script to do my own testing, which I run for 
every commit that touches QAPI:

#!/usr/bin/env bash
set -e

if [[ -f qapi/.flake8 ]]; then
    echo "flake8 --config=qapi/.flake8 qapi/"
    flake8 --config=qapi/.flake8 qapi/
fi
if [[ -f qapi/pylintrc ]]; then
    echo "pylint --rcfile=qapi/pylintrc qapi/"
    pylint --rcfile=qapi/pylintrc qapi/
fi
if [[ -f qapi/mypy.ini ]]; then
    echo "mypy --config-file=qapi/mypy.ini qapi/"
    mypy --config-file=qapi/mypy.ini qapi/
fi

if [[ -f qapi/.isort.cfg ]]; then
    pushd qapi
    echo "isort -c ."
    isort -c .
    popd
fi

pushd ../bin/git
make -j9
make check-qapi-schema
popd