python / typeshed

Collection of library stubs for Python, with static types
Other
4.36k stars 1.74k forks source link

`configparser.ConfigParser.values()` should return a `ValuesView[configparser.SectionProxy]` #11547

Open fizbin opened 7 months ago

fizbin commented 7 months ago

Consider these two similar python programs, which have identical behavior:

#! /usr/bin/env/python

import configparser
import re
import sys

if __name__ == "__main__":
    for filename in sys.argv[1:]:
        cparser = configparser.ConfigParser(interpolation=None)
        cparser.read(filename)
        changed = False
        for _ignored, section in cparser.items():
            if section.name == "DEFAULT":
                continue
            for k, v in section.items():
                if re.fullmatch(r"\d+", v):
                    section[k] = str(int(v) + 1)
                    changed = True
        if changed:
            with open(filename, "wt", encoding="utf-8") as fp:
                cparser.write(fp)

and

#! /usr/bin/env/python

import configparser
import re
import sys

if __name__ == "__main__":
    for filename in sys.argv[1:]:
        cparser = configparser.ConfigParser(interpolation=None)
        cparser.read(filename)
        changed = False
        for section in cparser.values():
            if section.name == "DEFAULT":
                continue
            for k, v in section.items():
                if re.fullmatch(r"\d+", v):
                    section[k] = str(int(v) + 1)
                    changed = True
        if changed:
            with open(filename, "wt", encoding="utf-8") as fp:
                cparser.write(fp)

(The only difference between the two programs is invoking cparser.items() or cparser.values())

The first program type-checks in mypy without any issues, but the second program:

/tmp/incints.py:13: error: "Mapping[str, str]" has no attribute "name"  [attr-defined]
/tmp/incints.py:17: error: Unsupported target for indexed assignment ("Mapping[str, str]")  [index]
Found 2 errors in 1 file (checked 1 source file)

The problem is that ConfigParser objects accept assignment of arbitrary str -> str mappings but when you pull a value back out of a ConfigParser, the value pulled out is always a configparser.SectionProxy. (see the __getitem__ and __setitem__ methods on configparser.RawConfigParser)

Note that this affects not only the return type of values but also the return type of __getitem__; it should be perfectly well-typed to write cparser[some_str_value].name.

srittau commented 7 months ago

I don't know the exact underlying problem, but in the stub we mimic the implementation by overriding items() and inheriting values() from MutableMapping. Potentially, we should derive from MutableMapping[str, SectionProxy], instead of MutableMapping[str, _Section], but I'm not sure whether that's the right solution or not.

fizbin commented 7 months ago

Potentially, we should derive from MutableMapping[str, SectionProxy], instead of MutableMapping[str, _Section], but I'm not sure whether that's the right solution or not.

It isn't. The issue is that for purposes of setting values, any Mapping[str, str] will work, but when you get a value, what you get back is always a SectionProxy.

See the implementation of these two methods on RawConfigParser, from which ConfigParser descends:

    def __getitem__(self, key):
        if key != self.default_section and not self.has_section(key):
            raise KeyError(key)
        return self._proxies[key]

    def __setitem__(self, key, value):
        # To conform with the mapping protocol, overwrites existing values in
        # the section.
        if key in self and self[key] is value:
            return
        # XXX this is not atomic if read_dict fails at any point. Then again,
        # no update method in configparser is atomic in this implementation.
        if key == self.default_section:
            self._defaults.clear()
        elif key in self._sections:
            self._sections[key].clear()
        self.read_dict({key: value})

I suspect that the solution is to, in the stub, override items, values and __getitem__ to return types based on SectionProxy.