fix(cli): scope config interpolation to radon section #252
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
radon reads and passes the entire contents of a configuration file to a parser, rather than just its section. This parser supports interpolation (or automatic substitution) for values using the '%' sign and will raise an error if they are not escaped (using '%%') or do not match the required pattern (
%\(([^)]+)\)s
) - see #244.While this is unlikely to cause an issue with standalone radon configuration files such as radon.cfg, with PEP 518 defining a common location in pyproject.toml for storing tool configuration in a
[tool]
table; users that install and run radon 6.0.0 onwards (with pyproject.toml support) will experience the aforementioned error if they are using '%' in any values in configuration sections that are unrelated to radon.Thus, the issue can be resolved by either removing interpolation functionality for radon configuration completely OR by performing it against the radon section only. This PR applies the latter, with the removal of interpolation as something that could be applied in a future radon major version. It thus resolves #244 and resolves #251.
Detail
radon uses
configparser.ConfigParser
for parsing configuration files. This class performs interpolation using the '%' sign and they must be escaped where not required for interpolation with '%%'. For example,%(base_dir)s/tests
would be substituted with the value of abase_dir
key in the same section, however the raw not-interpolated value could be attained by adding another '%' at its beginning. See the Python documentation for further detail. radon also passes the entire configuration file to the parser class - not just its section.radon 6.0.0 introduced pyproject.toml support and reads that file when using Python 3.11 (native toml support) - or earlier versions with
tomli
installed. Thus any values in pyproject.toml files that contain '%' signs and that do not match the interpolation pattern, will result in aValueError
forinvalid interpolation syntax
. This also holds true if such values are found in.ini
or.cfg
radon configuration files. However with PEP 518 defining a[tool]
table for configuration of multiple tools and many projects moving more configuration into pyproject.toml, there may be increased likelihood of this occurring, compared to previously.Solutions
There are different solutions available to eliminate this error for users.
Remove interpolation support
Removing interpolation support would prevent this error. This can be achieved by migrating the parser to
configparser.RawConfigParser
which does not perform substitutions.It could be argued that there are few or no elements of radon configuration that users could use with interpolation today; however as usage is challenging to ascertain, reserving deprecation for a future major version may be the safest option.
Other Python static analysis tooling such as pytest and black do not appear to use interpolation.
Limit interpolation to radon section only
The default interpolator used by
configparser.ConfigParser
interpolates only within each section - though support across sections can be achieved by usingconfigparser.ExtendedInterpolation
instead. Therefore the parser does not need to be aware of other sections and we can pass the radon configuration to it only - preventing errors caused by unrelated configuration.This can be achieved by using the raw configuration parser (
configparser.RawConfigParser
) mentioned in the previous section initially; and to then pass the radon section only to the current parser (configparser.ConfigParser
) - where it will be interpolated - preventing breaking changes.Removing the
if
clause in future would be the only change to deprecate interpolation - as mentioned in the previous section.The
read_dict
method is not available in Python 2's (ConfigParser.ConfigParser
) class and is thus incompatible. Importing thebuiltins
in theradon.cli.harvest
module is also not compatible with Python 2. If Python 2 support can be removed, a Python3-only deprecation warning could optionally be added as below to inform anyone using that feature of the future change. Though this PR focuses on addressing errors from unrelated configuration without deviating from the existing source code as much as possible; and deprecating Python 2 support may be more suitable as its own scoped issue due to the changes involved.Mitigation
For users experiencing the issue with a pyproject.toml file, the issue can be mitigated by rolling back to 5.1.0 (if not dependent on versions later than 6.0.0) - as it does not have support to check that file - and by moving any radon configuration to an independent radon.cfg configuration file.