Create and use a function for module stats initialization

[Pierre-Sassoulas]

Type of Changes

	Type
✓	🔨 Refactoring

Description

This permit to reduce the coupling between Pylinter and linterstats.

Refactor prior to #4720

[coveralls]

Pull Request Test Coverage Report for Build 1432495452

• 6 of 6 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 93.345%

---

Totals
Change from base Build 1431845150:	0.001%
Covered Lines:	13774
Relevant Lines:	14756

---

💛 - Coveralls

[DanielNoord]

Change seems fine, but why would we want this to be possible? I didn't really understand why this was problematic in #4720: don't we always want to set_current_module?

[Pierre-Sassoulas]

don't we always want to set_current_module?

Yes, it's better to set the current module (actually the functional result is a lot better after handling your comment properly with a nice output with the name of the config file). It's just a refactor to reduce coupling between Pylinter and pylint.util that felt better separated from the TOML changes.

[DanielNoord]

Shall we say something about that in the docstring? Like Method should normally be called through set_current_module?
I fear people might see this method and start accessing it directly without setting the current module. That could create problems in the future for function that do depend on setting the module.

[Pierre-Sassoulas]

I fear people might see this method and start accessing it directly without setting the current module. That could create problems in the future for function that do depend on setting the module.

It's probably a good thing that we can use stats from outside the Pylinter, for example if we want to create stats from each checker class (?) or from the OptionMixinManager without instancing a Pylinter. It's expected that you have to use the name again later to increase the module stats, right ? Worst case scenario there is a ModuleStats with zero violations in it ?

Maybe we could even create the ModuleStats when someone try to increment it for the first time ? This means the usage is simpler as you don't have to initialize it anymore : it's transparent for you.

[DanielNoord]

It's probably a good thing that we can use stats from outside the Pylinter, for example if we want to create stats from each checker class (?) or from the OptionMixinManager without instancing a Pylinter. It's expected that you have to use the name again later to increase the module stats, right ? Worst case scenario there is a ModuleStats with zero violations in it ?

I don't think you can use a checker class without instantiating a PyLinter anyway. You need methods like add_message for most of the methods and functions within those classes.

You will need the module_name to increase stats indeed, but I wonder if we can run into problems when you miss out on:

         self.reporter.on_set_current_module(modname, filepath)
         self.current_name = modname
         self.current_file = filepath or modname

Maybe we could even create the ModuleStats when someone try to increment it for the first time ? This means the usage is simpler as you don't have to initialize it anymore : it's transparent for you.

Perhaps we should. I'm not sure..
Personally I like that there current flow guarantees that you need to think about setting a module, creating the stats and then being able to increment it. Although it is more user-friendly to automate some of this, I like this more conscious thinking about the flow of things. But I leave this up to you, you're the maintainer :)

[Pierre-Sassoulas]

You need methods like add_message for most of the methods and functions within those classes.

Right, I thought there was an add_message in BaseChecker and there is... but it's calling the Pylinter. It's really everywhere. Which is why I try to remove responsibility from it.

I like this more conscious thinking about the flow of things.

I think letting the LinterStats class really handle the stats would remove complexity from Pylinter but it does seem like it's only used through Pylinter so it's not strictly necessary right now. Disentangling it would be theoretically beneficial but also eat up a lot of time and Pylinter would still be a god class handling everything. I'm going to simply add the comment you were suggesting. There's more efficient thing to do to make Pylinter more manageable. Let me know what you think :)

[DanielNoord]

@Pierre-Sassoulas Perfect!

The new Literal is a nice touch! 😄