duplicate-field-declaration (ODW9503)

Preview (since 0.16.3.33) · Related issues · View source

Derived from the odoo linter.

This rule is unstable and in preview. The --preview flag is required for use.

What it does

Checks for a field declared more than once in the body of the same Odoo model class.

Why is this bad?

A class body is executed top to bottom, so only the last assignment to a name survives in the class __dict__ the ORM reads. Every earlier declaration of the same field is dead: its comodel, its relation table, its compute and its label never reach the registry, and nothing reports that they were dropped.

The two declarations rarely agree. When they don't, the field that ends up in the database is the last one, which is the opposite of what a reader scanning the class from the top concludes. A Many2many whose dead declaration named a relation is the sharp case: that table is never created, while the code naming it still reads as if it were.

Declaring the same field again in a different file or module is not this. That is Odoo inheritance -- a module extending a model and overriding one of its fields -- and it is the supported way to change a field. Only a name bound twice inside one class body is reported, and a declaration is recognised by its fields.<Type>(...) call, the spelling Odoo models use.

Example

class ResPartner(models.Model):
    _inherit = "res.partner"

    category_ids = fields.Many2many(
        "res.partner.category.report",
        relation="res_partner_res_partner_category_report_rel",
    )
    category_ids = fields.Many2many(
        "res.partner.category",
        relation="res_partner_res_partner_category_rel",
    )

Use instead:

class ResPartner(models.Model):
    _inherit = "res.partner"

    category_ids = fields.Many2many(
        "res.partner.category",
        relation="res_partner_res_partner_category_rel",
    )

One diagnostic is reported per duplicated field, anchored on its first declaration and listing every other one, so a field declared three times reads as a single finding rather than as two. Removing the declarations that have no effect is the mechanically correct edit: the last one is what the ORM already read and what the database follows, so dropping the others cannot change how the code runs. No fix is offered even so. The duplicate is normally an accident, which means the surviving declaration is not necessarily the one anybody chose, and applying the edit automatically would settle that question without anybody looking at it.