[RFC] Tighten valid module version check

[naglis]

Currently, the following module versions will be considered as valid by pylint-odoo when using default configuration:

• 10+0.1.0.0, because the dot from valid_odoo_versions is not escaped when building the valid module version pattern (and the . inside a regex matches any character other than a newline):

  pylint-odoo/pylint_odoo/checkers/no_modules.py

  Lines 657 to 661 in a14ddca

  	def formatversion(self, string):
  	self.config.manifest_version_format_parsed = \
  	self.config.manifest_version_format % dict(
  	valid_odoo_versions='|'.join(self.config.valid_odoo_versions))
  	return re.match(self.config.manifest_version_format_parsed, string)

• 10.0.1.0.0WHATEVER, because re.match is used inside formatversion(), which matches the string from the beginning, however, there is no $ at the end of the default value for manifest_version_format to match the end of the string:

  pylint-odoo/pylint_odoo/checkers/no_modules.py

  Line 237 in a14ddca

  DFTL_MANIFEST_VERSION_FORMAT = r"(%(valid_odoo_versions)s)\.\d+\.\d+\.\d+"

  
  Thus, anything can follow once the version is matched from the beginning of the string.

I have started working on a fix, however, I have stumbled into a slight problem for which I need feedback from the authors - if a .pylintrc file is used with manifest_version_format defined inside it using the %(valid_odoo_versions)s placeholder, then manifest_version_format will be set automatically by ConfigParser (which pylint uses for configuration parsing) due to interpolation. This means that at the point when the manifest_version_format_parsed string is built:

pylint-odoo/pylint_odoo/checkers/no_modules.py

Lines 657 to 661 in a14ddca

	def formatversion(self, string):
	self.config.manifest_version_format_parsed = \
	self.config.manifest_version_format % dict(
	valid_odoo_versions='|'.join(self.config.valid_odoo_versions))
	return re.match(self.config.manifest_version_format_parsed, string)

then self.config.manifest_version_format no longer contains the %(valid_odoo_versions)s placeholder, as it was already formatted by ConfigParser. This becomes a problem if we want to escape the dots from valid_odoo_versions (eg. 10.0 -> 10\.0) inside formatversion(), as our values will be simply ignored if a config file is used with manifest_version_format defined inside it using the %(valid_odoo_versions)s placeholder.

I have comeup with one solution - switch to str.format() style formatting for manifest_version_format in the config, which would allow to avoid interpolation by ConfigParser. However, I guess it would be a breaking change. So my question is - would it be ok?

Maybe someone has other suggestions?

[moylop260]

We have a guideline from CONTRIBUTING.md#idioms:

• Prefer % over .format(), prefer %(varname) instead of positional. This is better for translation and clarity.

And we had issues related with this method in the past related with:

• 

E.g. OCA/manufacture#146

Maybe for pylint is not a real issue but we prefer to follow the same odoo guidelines here too.

What is the wrong with binop (%) vs format here?

[moylop260]

I have created the following fix: #153
Could you check it, please?

[naglis]

What is the wrong with binop (%) vs format here?

The problem is that the %()s placeholders are automaticaly formated by ConfigParser (due to interpolation). Thus, if a .pylintrc file is used with manifest_version_format defined inside it using the %(valid_odoo_versions)s placeholder these lines have no effect:

pylint-odoo/pylint_odoo/checkers/no_modules.py

Lines 658 to 660 in a14ddca

	self.config.manifest_version_format_parsed = \
	self.config.manifest_version_format % dict(
	valid_odoo_versions='|'.join(self.config.valid_odoo_versions))