[BUG][W8113]String parameter equal to name of variable (capital letters)

[victoralmau]

Related to: OCA/knowledge#357 (comment)

Use case:

There is a field

has_changes_pending_approval = fields.Boolean(
        string="Has changes pending approval"
)

The message "The attribute string is redundant. String parameter equal to name of variable" (W8113) is detected but it is incorrect because the string that the field had did NOT contain capital letters.

Expected behavior:
Only show the message if the existing string is exactly the same as the name of the variable with capital letters.

[antonag32]

@victoralmau Thanks for bringing this up.

I personally believe this is an edge case, and subject to interpretation.
The strings Has changes pending approval and Has Changes Pending Approval are clearly not the same, but the message they transmit is the same. Based on this, if you asked me about it, I would say the string is redundant.

I also believe that newbies may declare a field as in your example accidentally, meaning they don't know Odoo already generates automatic strings based on the field's name, instead of them actually wanting it to be shown without capitalization.

This means that if we were to make the change, pylint would not find as much redundant strings as it could.

However I can also see how this is clearly a bug, since the behavior is not expected. I think we should do the following:

• Document clearly what pylint considers redundant strings
• Update code to explicitly consider strings as redundant based on the definition above
• Use # pylint: disable=attribute-string-redundant in edge cases like yours.

And treat this the same as env.cr.commit(), pylint warns about it by default, since most people don't really need it, but the ones that really need it can just manually disable the warning, which also makes the intent clear, they know it is not recommended, but they also know what they are doing and so they disable it.

What do you think?

[pedrobaeza]

They are clearly not the same. We want to specifically use a non capital string, and why we have a pylint error in such case? The error should only appear when the string is exactly the one proposed by Odoo, casing included, which is not the case.

This is not an edge case. This happens a lot around modules, and forcing everyone to use the Odoo default casing is not correct.

Another example: you have number_of_records = fields.Integer(string="Number of Records") (or string="Number of records"). Why do you have to be forced to accept Number Of Records label?

[antonag32]

As expected there are differing opinions on the topic. Your point is good and after further thought and discussion with other teammates, we agree.

So unless someone else fixes it first, I hope to make the required changes in the near future.

Greetings 🤝

[pedrobaeza]

Thank you very much for the understanding and the future patch. We haven't touched the architecture of pylint-odoo, so for us it's more difficult to find which thing to modify.