Re: [PATCH] gnulib-tool.py: Add type hints to all functions.

[Bruno Haible]

Hi Collin,

> I am a big fan of the GNU C attributes for warnings, nonnull and
> warn_unused_result, for example. I assume most of the syntax
> strangeness is from remaining compliant with ISO C, correct?

I guess the strange syntax comes from
  - a wish to treat all ISO C extensions in a similar way syntactically,
    without extending the parser too much,
  - a lack of sense of aesthetics.

> For what it's worth, I would like to change that class around anyways.
> In most places it is used like an extended dictionary. One exception
> to this is GLImport.actioncmd():
> 
>        def actioncmd(self) -> str:
>         '''Return command-line invocation comment.'''
>         modules = self.config.getModules()
>         avoids = self.config.getAvoids()
>         destdir = self.config.getDestDir()
> 
> Here is GLImport.gnulib_cache() for comparison:
> 
>     def gnulib_cache(self) -> str:
>         '''omitted...'''
>         emit = ''
>         moduletable = self.moduletable
>         actioncmd = self.actioncmd()
>         localpath = self.config['localpath']
>         sourcebase = self.config['sourcebase']
> 
> The first one makes much more sense to me. For type hinting it is much
> clearer. It would remove most of the union stuff.

Be aware that the update / update_key / default / isdefault stuff was
created in order to make the reading of gnulib-cache.m4 files possible,
without too much boilerplate code.

> Since this is done in GLImport.actioncmd, maybe this was the goal
> anyways? Someone just didn't get around to finishing it?

I don't know. Before you start with that, take a deep look at the
gnulib-cache.m4 code.

> We would have to double check comparability with Python 3.7+, though I
> think it should be safe.

s/comparability/compatibility/
Yes, you'll need to check whether Python 3.7 supports Literal[...].

> becomes:
> 
>     def setLGPL(self, lgpl: Literal[None, True, '2', '3orGPLv2', '3']) -> 
> None:

Now, that is possibly overkill: It is very descriptive and clear, but
each time we add new possible option values for the --lgpl option, we
will need to adapt more places in the code.

> This should help various type checkers who will warn if there are
> paths that can be taken that break this rule.

True. But try to keep a balance between helping the type checkers and
keeping future maintenance simple.

> Therefore it would be:
> 
>      def __init__(self, config: GLConfig, transformers: dict[str, str] = 
> dict()):

Apparently, yes.

Bruno