Download tests/test_semantics.py from DBax127/nexa: direct link, hf CLI and curl.
- Browser
- Download file 112 kB
-
https://huggingface.co/DBax127/nexa/resolve/main/tests/test_semantics.py
- Command line
-
hf download hf://DBax127/nexa/tests/test_semantics.py
-
curl -L -o test_semantics.py https://huggingface.co/DBax127/nexa/resolve/main/tests/test_semantics.py
112 kB
| """The AST checkers, and the fragment gate that decides when to trust them. | |
| Each test names the defect it guards. Every one of them fails against the code | |
| as it stood before the session that wrote them. | |
| """ | |
| import inspect | |
| import pytest | |
| import corpus | |
| import semantics | |
| # ------------------------------------------------- prefer_sql_constraint_applies | |
| UNIQUENESS_IN_PYTHON = """@api.constrains('code') | |
| def _check_unique_code(self): | |
| for rec in self: | |
| if self.search_count([('code', '=', rec.code), ('id', '!=', rec.id)]): | |
| raise ValidationError('Code must be unique')""" | |
| SQL_CONSTRAINT = """_sql_constraints = [ | |
| ('code_uniq', 'UNIQUE(code)', 'Code must be unique.'), | |
| ]""" | |
| LITERAL_BOUND = """@api.constrains('product_uom_qty') | |
| def _check_quantity(self): | |
| for rec in self: | |
| if rec.product_uom_qty < 0: | |
| raise ValidationError('Quantity cannot be negative')""" | |
| CROSS_TABLE = """@api.constrains('product_id', 'product_uom_qty', 'company_id') | |
| def _check_available_qty(self): | |
| for rec in self: | |
| if not rec.product_id or rec.product_uom_qty <= 0: | |
| continue | |
| available = rec.product_id.with_company(rec.company_id).qty_available | |
| if rec.product_uom_qty > available: | |
| raise ValidationError('too much')""" | |
| TWO_COLUMN_COMPARISON = """@api.constrains('date_start', 'date_end') | |
| def _check_dates(self): | |
| for rec in self: | |
| if rec.date_start and rec.date_end and rec.date_end < rec.date_start: | |
| raise ValidationError('End before start')""" | |
| FLOAT_UTILS = """@api.constrains('selling_price') | |
| def _check_price(self): | |
| precision = self.env['decimal.precision'].precision_get('Product Price') | |
| for rec in self: | |
| if float_is_zero(rec.selling_price, precision_digits=precision): | |
| raise ValidationError('Price cannot be zero')""" | |
| def fires(code): | |
| return bool(semantics.prefer_sql_constraint_applies(code)) | |
| def test_uniqueness_in_python_is_caught(): | |
| """The rule's own 'wrong' sample. validate fails the build without this.""" | |
| assert fires(UNIQUENESS_IN_PYTHON) | |
| def test_sql_constraint_is_silent(): | |
| """The rule's own 'correct' sample. A checker that fires here cries wolf.""" | |
| assert not fires(SQL_CONSTRAINT) | |
| def test_literal_bound_is_caught(): | |
| """The gap that started it all. | |
| The predecessor regex matched only `@api.constrains ... search_count`, so a | |
| single-column bound -- the commonest case the rule governs -- passed twelve | |
| domain checks in silence. | |
| """ | |
| assert fires(LITERAL_BOUND) | |
| def test_decorator_call_does_not_disqualify_the_function(): | |
| """@api.constrains(...) is itself an ast.Call. | |
| Walking the whole function node counts it among the body's calls, and since | |
| any call disqualifies a candidate, every constraint disqualified itself. The | |
| checker looked correct and caught nothing. | |
| """ | |
| assert fires(LITERAL_BOUND) | |
| assert semantics.prefer_sql_constraint_applies(LITERAL_BOUND)[0][0] == 4 | |
| def test_cross_table_access_is_silent(): | |
| """A CHECK genuinely cannot read another table, so this must not fire. | |
| Verified against real 30B output, not an invented sample: the reach through | |
| rec.product_id.with_company(...) is what puts it out of scope. | |
| """ | |
| assert not fires(CROSS_TABLE) | |
| def test_two_column_comparison_is_silent(): | |
| """A declared limit, asserted so it cannot be lost by accident. | |
| CHECK(date_end >= date_start) is valid SQL, so this is recall deliberately | |
| traded for precision on a `high` rule. If someone later widens the checker, | |
| this test should fail and make them say so on purpose. | |
| """ | |
| assert not fires(TWO_COLUMN_COMPARISON) | |
| def test_helper_call_in_the_condition_is_silent(): | |
| assert not fires(FLOAT_UTILS) | |
| def test_message_only_field_read_does_not_block_a_finding(): | |
| """rec.display_name is read to phrase the error, not to decide the rule.""" | |
| code = """@api.constrains('qty') | |
| def _check_qty(self): | |
| for rec in self: | |
| if rec.qty <= 0: | |
| raise ValidationError('Bad qty on %s' % rec.display_name)""" | |
| assert fires(code) | |
| def test_unparseable_code_yields_no_findings(): | |
| assert semantics.prefer_sql_constraint_applies("def broken(:") == [] | |
| # ---------------------------------------------------------- is_lifted_fragment | |
| def test_method_taking_self_is_a_lifted_fragment(): | |
| """A method shown without its class was never going to carry its imports. | |
| Flagging them turned a correct answer into WARN and buried the real finding | |
| under two that could never have been otherwise. | |
| """ | |
| tree, _ = semantics.parse_python(LITERAL_BOUND) | |
| assert semantics.is_lifted_fragment(tree, []) | |
| def test_real_module_is_not_a_fragment(): | |
| """The gate must not disable the import check wholesale.""" | |
| code = """from odoo import api | |
| def helper(x): | |
| return ValidationError(x)""" | |
| tree, _ = semantics.parse_python(code) | |
| assert not semantics.is_lifted_fragment(tree, []) | |
| assert [m for _, m in semantics.missing_imports(tree)] | |
| def test_repair_notes_alone_mark_a_fragment(): | |
| tree, _ = semantics.parse_python("x = 1") | |
| assert semantics.is_lifted_fragment(tree, ["the block only parses as a class body"]) | |
| # ------------------------------------------------------------ the other checks | |
| def test_constrains_missing_field_is_caught(): | |
| code = """@api.constrains('date_start') | |
| def _check_dates(self): | |
| for rec in self: | |
| if rec.date_end < rec.date_start: | |
| raise ValidationError('End before start')""" | |
| hits = semantics.constrains_lists_every_field_read(code) | |
| assert any("date_end" in m for _, m in hits) | |
| def test_constrains_listing_every_field_is_silent(): | |
| assert semantics.constrains_lists_every_field_read(TWO_COLUMN_COMPARISON) == [] | |
| def test_depends_missing_dotted_path_is_caught(): | |
| code = """@api.depends('line_ids') | |
| def _compute_total(self): | |
| for rec in self: | |
| rec.total = sum(rec.line_ids.price)""" | |
| hits = semantics.depends_covers_compute_reads(code) | |
| assert any("line_ids.price" in m for _, m in hits) | |
| def test_every_registered_checker_is_callable_and_returns_triples(name): | |
| """run_check raises on an unknown name rather than passing silently. | |
| A renamed checker that returned 'clean' would turn a critical rule into a | |
| permanent false pass, so the registry is part of the contract. | |
| The third element is the likelihood a finding earned from a trigger, or None | |
| to keep its rule's grade. run_check normalises it, so a checker with no | |
| opinion about triggers still answers in the one shape every caller unpacks. | |
| """ | |
| out = semantics.run_check(name, LITERAL_BOUND) | |
| assert isinstance(out, list) | |
| for item in out: | |
| assert len(item) == 3 | |
| assert isinstance(item[0], int) and isinstance(item[1], str) | |
| assert item[2] is None or item[2] in corpus.LIKELIHOOD_ORDER | |
| def test_a_context_checker_is_registered_and_takes_facts(name): | |
| """CONTEXT_CHECKS is a hand-written set, so it can name a checker that no | |
| longer takes facts -- which would pass the tree to a positional argument | |
| meaning something else.""" | |
| assert name in semantics.CHECKS | |
| argspec = inspect.getfullargspec(semantics.CHECKS[name]) | |
| assert argspec.args[:2] == ["code", "facts"] | |
| def test_unknown_checker_name_raises(): | |
| with pytest.raises(KeyError): | |
| semantics.run_check("no_such_checker", "x = 1") | |
| # ------------------------------------------------- ondelete, read through | |
| ONDELETE_READ = """class Thing(models.Model): | |
| _name = "my.thing" | |
| task_id = fields.Many2one("my.task") | |
| def _label(self): | |
| for rec in self: | |
| rec.name = rec.task_id.filepath | |
| """ | |
| ONDELETE_DECLARED_ONLY = """class Thing(models.Model): | |
| _name = "my.thing" | |
| task_id = fields.Many2one("my.task") | |
| """ | |
| def test_a_dereferenced_many2one_without_ondelete_is_caught(): | |
| """The shape where 'set null' silently changes an answer: delete the parent, | |
| the reference is nulled, and rec.task_id.filepath becomes False.""" | |
| hits = semantics.ondelete_leaves_a_dangling_read(ONDELETE_READ) | |
| assert hits, "no finding" | |
| assert "filepath" in hits[0][1], "name the read that makes it matter" | |
| assert hits[0][0] == 4, "the line has to point at the declaration" | |
| def test_a_declaration_nobody_reads_through_is_silent(): | |
| """Nulling a reference no code dereferences changes no answer. Flagging it | |
| produced 4.1 findings per 1000 lines on reviewed OCA code.""" | |
| assert semantics.ondelete_leaves_a_dangling_read(ONDELETE_DECLARED_ONLY) == [] | |
| def test_required_many2one_is_exempt_from_ondelete(): | |
| """Odoo defaults a required Many2one to 'restrict', which is what the rule | |
| recommends, so the orphaning cannot happen there.""" | |
| code = ONDELETE_READ.replace('fields.Many2one("my.task")', | |
| 'fields.Many2one("my.task", required=True)') | |
| assert semantics.ondelete_leaves_a_dangling_read(code) == [] | |
| def test_related_many2one_is_exempt_from_ondelete(): | |
| code = ONDELETE_READ.replace('fields.Many2one("my.task")', | |
| 'fields.Many2one("my.task", related="x.task_id")') | |
| assert semantics.ondelete_leaves_a_dangling_read(code) == [] | |
| def test_a_declared_ondelete_is_silent(): | |
| code = ONDELETE_READ.replace('fields.Many2one("my.task")', | |
| 'fields.Many2one("my.task", ondelete="restrict")') | |
| assert semantics.ondelete_leaves_a_dangling_read(code) == [] | |
| def test_reading_the_relation_itself_is_not_a_dereference(): | |
| """`if rec.task_id:` is a recordset test and is falsy-safe. It is | |
| `rec.task_id.filepath` that quietly becomes False.""" | |
| code = """class Thing(models.Model): | |
| task_id = fields.Many2one("my.task") | |
| def _label(self): | |
| for rec in self: | |
| if rec.task_id: | |
| rec.name = "yes" | |
| """ | |
| assert semantics.ondelete_leaves_a_dangling_read(code) == [] | |
| def test_a_read_through_mapped_stays_unread(): | |
| """Measured and not taken (ARCHITECTURE 3.79). mapped("task_id.filepath") | |
| reads through the relation as the attribute does, but a read is placed by | |
| the field's name, and a mapped() is often called on records of another | |
| model: on the three trees it added one finding, a quant's location_id read | |
| in stock.location's file, and nothing true.""" | |
| code = ONDELETE_READ.replace( | |
| "rec.name = rec.task_id.filepath", | |
| 'rec.name = ",".join(self.mapped("task_id.filepath"))') | |
| assert semantics.ondelete_leaves_a_dangling_read(code) == [] | |
| # ----------------------------------------------------- check_company scope | |
| COMPANY_SCOPED = """class Thing(models.Model): | |
| _name = "my.thing" | |
| company_id = fields.Many2one("res.company", required=True) | |
| journal_id = fields.Many2one("account.journal") | |
| """ | |
| def test_a_company_scoped_model_needs_check_company(): | |
| hits = semantics.check_company_missing_on_company_owned_relation(COMPANY_SCOPED) | |
| assert hits, "no finding" | |
| assert "account.journal" in hits[0][1] | |
| def test_check_company_present_is_silent(): | |
| code = COMPANY_SCOPED.replace('fields.Many2one("account.journal")', | |
| 'fields.Many2one("account.journal", check_company=True)') | |
| assert semantics.check_company_missing_on_company_owned_relation(code) == [] | |
| def test_a_model_with_no_company_is_not_asked_for_check_company(): | |
| """check_company compares the record's company against the target's. On a | |
| model that has no company_id there is nothing to compare, and the regex | |
| that preceded this could not see the class at all.""" | |
| code = COMPANY_SCOPED.replace( | |
| ' company_id = fields.Many2one("res.company", required=True)\n', "") | |
| assert semantics.check_company_missing_on_company_owned_relation(code) == [] | |
| def test_res_company_itself_is_exempt(): | |
| """Odoo's own fields.py carries a branch for check_company on a field of | |
| res.company. Six of the 37 OCA hits were this.""" | |
| code = COMPANY_SCOPED.replace('_name = "my.thing"', '_inherit = "res.company"') | |
| assert semantics.check_company_missing_on_company_owned_relation(code) == [] | |
| def test_a_wizard_is_exempt(): | |
| """A TransientModel's records do not persist, and the harm the rule names is | |
| a multi-company client seeing another tenant's data on a DOCUMENT. Eleven of | |
| the 37 OCA hits were wizards.""" | |
| code = COMPANY_SCOPED.replace("models.Model", "models.TransientModel") | |
| assert semantics.check_company_missing_on_company_owned_relation(code) == [] | |
| def test_a_bare_declaration_block_is_still_checked(): | |
| """A generated answer arrives with no class around it. Treating the module | |
| as one implicit model body keeps the rule working on snippets, and costs | |
| nothing on real code, where fields always live in a class.""" | |
| code = """company_id = fields.Many2one("res.company", required=True) | |
| journal_id = fields.Many2one("account.journal") | |
| """ | |
| assert semantics.check_company_missing_on_company_owned_relation(code) | |
| # --------------------------------------------- storedness, and underscores | |
| def test_a_non_stored_compute_is_outside_the_rule(): | |
| """The rule is about STORED computes -- "written once and never recomputed, | |
| survives restarts". None of that happens to a value the database never | |
| holds. 30 of 95 OCA findings were non-stored computes.""" | |
| code = """total = fields.Float(compute="_compute_total") | |
| @api.depends('line_ids') | |
| def _compute_total(self): | |
| for rec in self: | |
| rec.total = sum(l.price for l in rec.line_ids) | |
| """ | |
| assert semantics.depends_covers_compute_reads(code) == [] | |
| def test_a_stored_compute_is_still_checked(): | |
| code = """total = fields.Float(compute="_compute_total", store=True) | |
| @api.depends('line_ids') | |
| def _compute_total(self): | |
| for rec in self: | |
| rec.total = sum(l.price for l in rec.line_ids) | |
| """ | |
| hits = semantics.depends_covers_compute_reads(code) | |
| assert any("line_ids.price" in m for _, m in hits) | |
| def test_an_undeclared_compute_keeps_the_rules_assumption(): | |
| """Most of this checker's work is on generated snippets, where no field | |
| declaration is present at all. Absence is unknown, not unstored.""" | |
| code = """@api.depends('line_ids') | |
| def _compute_total(self): | |
| for rec in self: | |
| rec.total = sum(l.price for l in rec.line_ids) | |
| """ | |
| assert semantics.depends_covers_compute_reads(code) | |
| # A compute that fills its field through a helper, update() or write() makes | |
| # no attribute store, so the field it produces used to read as a dependency. | |
| # calendar.recurrence's _compute_rrule compares and write()s its own rrule; | |
| # OpenSPP's _compute_center_area_ids clears and fills its own through update(). | |
| OWN_OUTPUT = """class Recurrence(models.Model): | |
| rrule = fields.Char(compute="_compute_rrule", store=True) | |
| rrule_type = fields.Selection([("daily", "Daily")]) | |
| label = fields.Char(compute="_compute_label", store=True) | |
| @api.depends("rrule_type") | |
| def _compute_rrule(self): | |
| for recurrence in self: | |
| current = recurrence._serialize() | |
| if recurrence.rrule != current: | |
| recurrence.write({"rrule": current}) | |
| """ | |
| def test_a_field_the_file_says_the_method_computes_is_its_own_output(): | |
| assert semantics.run_check("depends_covers_compute_reads", OWN_OUTPUT) == [] | |
| def test_another_computed_field_or_the_same_one_through_a_relation_still_counts(): | |
| """The exemption is the method's own output on the same record. A field | |
| another method computes is an input, and rrule read off a parent is a | |
| recursive dependency, which Odoo supports and wants declared.""" | |
| code = OWN_OUTPUT.replace( | |
| "if recurrence.rrule != current:", | |
| "if recurrence.rrule != current or recurrence.label or recurrence.parent_id.rrule:") | |
| reads = sorted(message.split("'")[1] for _, message, _ in | |
| semantics.run_check("depends_covers_compute_reads", code)) | |
| assert reads == ["label", "parent_id", "parent_id.rrule"], reads | |
| def test_a_read_through_the_methods_own_output_stays_exempt(): | |
| """Measured and kept (ARCHITECTURE 3.73). Odoo drops @api.depends("team_id") | |
| from team_id's own triggers but keeps "team_id.member_ids", so this read | |
| could be declared. With the exemption narrowed to the bare field, core | |
| gained 37 findings: 17 true, 16 of them a smart default like this one, | |
| checking its own earlier choice, and 20 false.""" | |
| code = """class Lead(models.Model): | |
| team_id = fields.Many2one("crm.team", compute="_compute_team_id", store=True, readonly=False) | |
| @api.depends("user_id") | |
| def _compute_team_id(self): | |
| for lead in self: | |
| if lead.team_id and lead.user_id in lead.team_id.member_ids: | |
| continue | |
| lead.team_id = lead._default_team(lead.user_id) | |
| """ | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [] | |
| def test_an_underscore_name_is_not_a_field(): | |
| """Odoo reserves the underscore prefix for ORM internals and mixin | |
| configuration. base_time_window reads self._time_window_overlap_check_field, | |
| and the checker told its author to declare it in @api.constrains, where the | |
| ORM ignores it.""" | |
| code = """@api.constrains('start', 'end') | |
| def _check_window(self): | |
| for rec in self: | |
| field = rec._time_window_overlap_check_field | |
| if rec.start > rec.end: | |
| raise ValidationError('bad') | |
| """ | |
| hits = semantics.constrains_lists_every_field_read(code) | |
| assert not any("_time_window" in m for _, m in hits) | |
| # mapped("a.b") names a field without an attribute access ever appearing. The | |
| # checker used to read only the receiver, so this compute looked declared and | |
| # the same read spelled as a generator was reported. | |
| MAPPED_COMPUTE = """@api.depends({declared}) | |
| def _compute_outstanding_total(self): | |
| for account in self: | |
| account.outstanding_total = sum(account.invoice_ids.{call}) | |
| """ | |
| def test_a_mapped_path_is_a_read_the_depends_must_declare(): | |
| code = MAPPED_COMPUTE.format(declared='"invoice_ids"', | |
| call='mapped("amount_residual")') | |
| hits = semantics.run_check("depends_covers_compute_reads", code) | |
| assert [(line, "'invoice_ids.amount_residual'" in message) | |
| for line, message, _ in hits] == [(4, True)], hits | |
| def test_a_mapped_path_the_depends_declares_is_quiet(): | |
| for declared in ('"invoice_ids.amount_residual"', | |
| '"invoice_ids.amount_residual.currency_id"'): | |
| code = MAPPED_COMPUTE.format(declared=declared, | |
| call='mapped("amount_residual")') | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], \ | |
| declared | |
| def test_a_mapped_path_keeps_the_checkers_exemptions(): | |
| """A trailing .id names the relation, and a filtered() receiver is still | |
| the recordset it filters.""" | |
| code = MAPPED_COMPUTE.format( | |
| declared='"invoice_ids.partner_id"', | |
| call='filtered(lambda i: i.posted).mapped("partner_id.id")') | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [] | |
| def test_a_mapped_lambda_or_name_is_not_guessed(): | |
| for call in ("mapped(lambda i: i.amount_residual)", "mapped(FIELD)"): | |
| code = MAPPED_COMPUTE.format(declared='"invoice_ids"', call=call) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], call | |
| # mapped()'s siblings name what they read the same way, and were silent after | |
| # it was not. Each call below reads the paths beside it, relative to | |
| # invoice_ids; a trailing id names the relation and is never asked for. | |
| NAMED_READS = [ | |
| ('filtered("is_paid")', ["is_paid"]), | |
| ('filtered("partner_id.is_company")', ["partner_id.is_company"]), | |
| ('grouped("journal_id")', ["journal_id"]), | |
| ('sorted("sequence")', ["sequence"]), | |
| ('sorted(key="sequence", reverse=True)', ["sequence"]), | |
| ('sorted("date desc, id")', ["date"]), | |
| ('sorted("partner_id.name DESC NULLS LAST, date:month")', ["partner_id.name", "date"]), | |
| ('filtered_domain([("state", "=", "done"), "|", (1, "=", 1), ' | |
| '("partner_id.id", "=", 7)])', ["partner_id", "state"]), | |
| ('filtered_domain([("line_ids", "any", [("paid", "=", True)])])', | |
| ["line_ids", "line_ids.paid"]), | |
| ] | |
| def test_a_field_named_in_a_string_is_a_read_the_depends_must_declare(call, reads): | |
| code = MAPPED_COMPUTE.format(declared='"invoice_ids"', call=call) | |
| hits = semantics.run_check("depends_covers_compute_reads", code) | |
| reported = sorted(message.split("'")[1] for _, message, _ in hits) | |
| assert reported == sorted("invoice_ids." + read for read in reads), hits | |
| def test_a_field_named_in_a_string_the_depends_declares_is_quiet(call, reads): | |
| declared = ", ".join('"invoice_ids.{0}"'.format(read) for read in reads) | |
| code = MAPPED_COMPUTE.format(declared=declared, call=call) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [] | |
| def test_a_field_not_written_as_a_literal_is_not_guessed(call): | |
| code = MAPPED_COMPUTE.format(declared='"invoice_ids"', call=call) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], call | |
| NAMED_EXEMPT = """{head} | |
| @api.depends("invoice_ids", "line_ids.partner_id.name") | |
| def _compute_total(self): | |
| for rec in self: | |
| {body} | |
| """ | |
| def test_a_field_named_in_a_string_keeps_the_checkers_exemptions(head, body): | |
| code = NAMED_EXEMPT.format(head=head, body=body) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], body | |
| def test_a_named_display_name_override_is_left_alone(): | |
| code = """@api.depends("invoice_ids") | |
| def _compute_display_name(self): | |
| for rec in self: | |
| rec.display_name = ", ".join(rec.invoice_ids.sorted("date").mapped("name")) | |
| """ | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [] | |
| # A subscript stopped the path. rec.line_ids[0].amount read nothing past | |
| # line_ids, so a compute taking its value from the first line looked declared. | |
| SUBSCRIPT_COMPUTE = """@api.depends({declared}) | |
| def _compute_first(self): | |
| for rec in self: | |
| rec.first = {read} | |
| """ | |
| def test_a_read_after_a_subscript_by_position_is_a_read(read, path): | |
| code = SUBSCRIPT_COMPUTE.format(declared='"line_ids"', read=read) | |
| hits = semantics.run_check("depends_covers_compute_reads", code) | |
| assert ["'{0}'".format(path) in message for _, message, _ in hits] == [True], hits | |
| code = SUBSCRIPT_COMPUTE.format(declared='"{0}"'.format(path), read=read) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], read | |
| def test_a_subscript_reads_every_segment_of_the_chain_after_it(): | |
| """rec.partner_id.child_ids[:1].email reported partner_id.child_ids and | |
| never the email, which is the field whose change goes unnoticed.""" | |
| code = SUBSCRIPT_COMPUTE.format(declared='"partner_id"', | |
| read="rec.partner_id.child_ids[:1].email") | |
| hits = semantics.run_check("depends_covers_compute_reads", code) | |
| assert sorted(message.split("'")[1] for _, message, _ in hits) == [ | |
| "partner_id.child_ids", "partner_id.child_ids.email"], hits | |
| def test_a_loop_over_a_slice_binds_the_same_path(): | |
| code = """@api.depends("line_ids") | |
| def _compute_rest(self): | |
| for rec in self: | |
| for line in rec.line_ids.sorted("sequence")[1:]: | |
| rec.rest += line.discount | |
| """ | |
| hits = semantics.run_check("depends_covers_compute_reads", code) | |
| assert sorted(message.split("'")[1] for _, message, _ in hits) == [ | |
| "line_ids.discount", "line_ids.sequence"], hits | |
| def test_a_subscript_that_does_not_pick_records_by_position_is_not_followed(read): | |
| code = SUBSCRIPT_COMPUTE.format(declared='"line_ids"', read=read) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], read | |
| # A call stopped the path the same way. rec.sudo().partner_id.name read nothing | |
| # past the call, so a compute taking a partner's name through sudo() looked as | |
| # if it read nothing from the partner (ARCHITECTURE 3.78). | |
| def test_a_read_through_a_call_that_keeps_the_records_is_a_read(read, declared, path): | |
| code = SUBSCRIPT_COMPUTE.format(declared='"{0}"'.format(declared), read=read) | |
| hits = semantics.run_check("depends_covers_compute_reads", code) | |
| assert ["'{0}'".format(path) in message for _, message, _ in hits] == [True], hits | |
| code = SUBSCRIPT_COMPUTE.format(declared='"{0}"'.format(path), read=read) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], read | |
| def test_a_read_through_any_other_call_is_not_followed(read): | |
| code = SUBSCRIPT_COMPUTE.format(declared='"line_ids"', read=read) | |
| assert semantics.run_check("depends_covers_compute_reads", code) == [], read | |
| def test_a_constraint_reading_through_sudo_must_list_the_field(): | |
| code = NAMED_CONSTRAINT.format( | |
| declared='"date_start"', | |
| body='if self.sudo().date_end:\n raise ValidationError("x")') | |
| hits = semantics.run_check("constrains_lists_every_field_read", code) | |
| assert ["reads 'date_end'" in message for _, message, _ in hits] == [True], hits | |
| def test_a_read_through_a_name_assigned_records_stays_unread(): | |
| """Measured and not taken (ARCHITECTURE 3.80). pending is the first | |
| pending review exactly, and binding every name whose assignments all | |
| resolve to one path is exact too. What the depends check then reads | |
| through such names is mostly not a dependency: on odoo core 37 of 113 | |
| findings read true, 28 of them smart defaults, and the false ones were | |
| values kept on purpose, fields stored nowhere, and decorators merged or | |
| covered from outside the file. The tree facts decide 27 of the 113 now, | |
| and at most 37 of the 86 left can be true (ARCHITECTURE 3.87).""" | |
| depends = """@api.depends('review_ids.status', 'review_ids.sequence') | |
| def _compute_current_tier(self): | |
| for rec in self: | |
| pending = rec.review_ids.filtered(lambda r: r.status == 'pending').sorted('sequence')[:1] | |
| rec.current_tier_id = pending.tier_id | |
| """ | |
| constrains = """@api.constrains('is_default') | |
| def _check_unique_default(self): | |
| defaults = self.filtered('is_default') | |
| companies = {rec.id: rec.company_id.id for rec in defaults} | |
| if len(set(companies.values())) < len(companies): | |
| raise ValidationError('bad') | |
| """ | |
| assert semantics.run_check("depends_covers_compute_reads", depends) == [] | |
| assert semantics.run_check("constrains_lists_every_field_read", constrains) == [] | |
| # A constraint reads a field through a string as a compute does, and writing | |
| # that field alone skips the check. Only the root is compared, as for every | |
| # other read here: @api.constrains cannot follow a dotted name. | |
| NAMED_CONSTRAINT = """@api.constrains({declared}) | |
| def _check_dates(self): | |
| {body} | |
| """ | |
| def test_a_constraint_reading_a_field_through_a_string_must_list_it(body, root): | |
| code = NAMED_CONSTRAINT.format(declared='"date_start"', body=body) | |
| hits = semantics.run_check("constrains_lists_every_field_read", code) | |
| assert ["reads '{0}'".format(root) in message for _, message, _ in hits] == [True], hits | |
| code = NAMED_CONSTRAINT.format(declared='"date_start", "{0}"'.format(root), body=body) | |
| assert semantics.run_check("constrains_lists_every_field_read", code) == [], body | |
| def test_a_constraint_naming_a_field_only_in_its_message_is_quiet(): | |
| code = NAMED_CONSTRAINT.format( | |
| declared='"date_start"', | |
| body='if self.date_start:\n' | |
| ' raise ValidationError(", ".join(self.mapped("name")))') | |
| assert semantics.run_check("constrains_lists_every_field_read", code) == [] | |
| # ------------------------------------------------------ what a lambda reads | |
| def test_a_read_through_a_lambda_parameter_stays_unread(): | |
| """Measured and not taken (ARCHITECTURE 3.72). Binding the parameter is | |
| exact: filtered() hands its lambda every record of the receiver. But what | |
| a filter tests is mostly a field that selects records rather than one that | |
| feeds the value: on odoo core 40 of 127 depends findings read true, 28 of | |
| them smart defaults, and 9 of 27 constrains findings. The tree facts decide | |
| 32 of the 121 depends findings left since then, and at most 41 of the 89 | |
| they leave can be true (ARCHITECTURE 3.87).""" | |
| depends = """@api.depends('line_ids.amount') | |
| def _compute_total(self): | |
| for rec in self: | |
| rec.total = sum(rec.line_ids.filtered(lambda l: l.state == 'done').mapped('amount')) | |
| """ | |
| constrains = """@api.constrains('date_start') | |
| def _check(self): | |
| if self.filtered(lambda r: r.date_end and r.date_end < r.date_start): | |
| raise ValidationError('bad') | |
| """ | |
| assert semantics.run_check("depends_covers_compute_reads", depends) == [] | |
| assert semantics.run_check("constrains_lists_every_field_read", constrains) == [] | |
| # ------------------------------------------- narrowed by `nexa recall --aim` | |
| # Placed against the method each fix edited, prefer-sql-constraint scored 0 | |
| # on-target against 47 other-symbol and ondelete 1 against 34 -- the two worst | |
| # in the corpus. These pin the narrowings that came out of reading their | |
| # findings on 229,600 lines of OpenSPP. | |
| def test_uniqueness_across_another_model_is_not_this_rule(): | |
| """`self.env["other.model"].search(...)` tests uniqueness across tables, and | |
| a UNIQUE on THIS table cannot express it. Recommending one is advice that | |
| cannot be followed, which is worse than advice nobody wanted.""" | |
| code = ''' | |
| class Handler(models.Model): | |
| _name = "endpoint.route.handler" | |
| @api.constrains("route") | |
| def _check_route_unique_across_models(self): | |
| for model in self._consumer_models(): | |
| if self.env[model].sudo().search_count([("route", "in", self.routes)]): | |
| raise ValidationError("Non unique route") | |
| ''' | |
| assert semantics.run_check("prefer_sql_constraint_applies", code) == [] | |
| def test_uniqueness_on_self_is_still_reported(): | |
| """The narrowing must not cost the case the rule is for.""" | |
| code = ''' | |
| class Client(models.Model): | |
| _name = "api.client" | |
| @api.constrains("client_id") | |
| def _check_unique_client_id(self): | |
| for record in self: | |
| if self.search([("client_id", "=", record.client_id), | |
| ("id", "!=", record.id)], limit=1): | |
| raise ValidationError("Client ID must be unique") | |
| ''' | |
| found = semantics.run_check("prefer_sql_constraint_applies", code) | |
| assert len(found) == 1 | |
| assert "cannot race" in found[0][1] | |
| def test_a_class_that_already_declares_the_constraint_is_left_alone(): | |
| """Telling an author to do the thing they did is how a checker gets | |
| switched off. Both spellings: _sql_constraints, and the models.Constraint | |
| form Odoo 17 introduced.""" | |
| modern = ''' | |
| class Client(models.Model): | |
| _name = "api.client" | |
| _client_id_unique = models.Constraint("UNIQUE(client_id)", "Must be unique.") | |
| @api.constrains("client_id") | |
| def _check_unique_client_id(self): | |
| for record in self: | |
| if self.search([("client_id", "=", record.client_id)], limit=1): | |
| raise ValidationError("Client ID must be unique") | |
| ''' | |
| classic = modern.replace( | |
| '_client_id_unique = models.Constraint("UNIQUE(client_id)", "Must be unique.")', | |
| '_sql_constraints = [("client_id_uniq", "UNIQUE(client_id)", "Must be unique.")]') | |
| assert semantics.run_check("prefer_sql_constraint_applies", modern) == [] | |
| assert semantics.run_check("prefer_sql_constraint_applies", classic) == [] | |
| def test_a_constraint_on_a_different_column_does_not_excuse_this_one(): | |
| """The narrowing is per column. A model guarding `code` has not guarded | |
| `client_id`, and treating the class as covered would lose real findings.""" | |
| code = ''' | |
| class Client(models.Model): | |
| _name = "api.client" | |
| _code_unique = models.Constraint("UNIQUE(code)", "Code must be unique.") | |
| @api.constrains("client_id") | |
| def _check_unique_client_id(self): | |
| for record in self: | |
| if self.search([("client_id", "=", record.client_id)], limit=1): | |
| raise ValidationError("Client ID must be unique") | |
| ''' | |
| assert len(semantics.run_check("prefer_sql_constraint_applies", code)) == 1 | |
| ONDELETE_BODY = ''' | |
| class Alert(models.Model): | |
| _name = "spp.alert" | |
| model_id = fields.Many2one("ir.model") | |
| def run(self): | |
| {body} | |
| ''' | |
| def test_a_guarded_dereference_is_not_a_dangling_read(): | |
| """The rule's complaint is that a nulled reference yields False rather than | |
| raising. An author who tested for it has answered that, and on OpenSPP the | |
| guard is the majority idiom: 101 findings became 20 once it was read.""" | |
| for guard in ( | |
| " if not self.model_id:\n return 0\n" | |
| " return self.model_id.model", | |
| " if self.model_id:\n return self.model_id.model", | |
| " return self.model_id and self.model_id.model or ''", | |
| " return self.model_id.model if self.model_id else ''"): | |
| code = ONDELETE_BODY.format(body=guard) | |
| assert semantics.run_check("ondelete_leaves_a_dangling_read", code) == [], guard | |
| def test_an_unguarded_dereference_is_still_reported(): | |
| """The narrowing must not cost the case the rule is for: a compute that | |
| reads straight through the relation and silently writes False.""" | |
| code = ONDELETE_BODY.format(body=" return self.model_id.model") | |
| found = semantics.run_check("ondelete_leaves_a_dangling_read", code) | |
| assert len(found) == 1 | |
| assert "set null" in found[0][1] | |
| def test_a_guard_in_one_method_does_not_excuse_another(): | |
| """Function-level, not file-level. A method that checks the field says | |
| nothing about the method next to it that does not.""" | |
| code = ''' | |
| class Alert(models.Model): | |
| _name = "spp.alert" | |
| model_id = fields.Many2one("ir.model") | |
| def careful(self): | |
| if not self.model_id: | |
| return 0 | |
| return self.model_id.model | |
| def careless(self): | |
| return self.model_id.name | |
| ''' | |
| found = semantics.run_check("ondelete_leaves_a_dangling_read", code) | |
| assert len(found) == 1 | |
| def test_an_unproven_advisory_rule_still_fires_correctly(): | |
| """odoo.except-order-follows-the-hierarchy ships advisory because nothing | |
| measured here has met the defect, not because the defect is imaginary. The | |
| checker still has to work, or the label is covering for a broken rule.""" | |
| shadowed = ( | |
| "try:\n" | |
| " record.action_approve()\n" | |
| "except UserError as exc:\n" | |
| " pass\n" | |
| "except AccessError as exc:\n" | |
| " pass\n") | |
| found = semantics.run_check("handler_order_shadows_a_subclass", shadowed) | |
| assert len(found) == 1 | |
| assert "never runs" in found[0][1] | |
| assert semantics.run_check( | |
| "handler_order_shadows_a_subclass", | |
| shadowed.replace("UserError", "_TMP").replace("AccessError", "UserError") | |
| .replace("_TMP", "AccessError")) == [] | |
| def test_the_odoo_hierarchy_matches_odoo_exceptions(): | |
| """Read out of odoo/exceptions.py, identical in 16.0 and 18.0. If Odoo ever | |
| reparents one of these the rule is wrong, and this is where that shows.""" | |
| for name in ("AccessDenied", "AccessError", "MissingError", "ValidationError"): | |
| assert semantics._is_subclass_of(name, "UserError"), name | |
| assert not semantics._is_subclass_of("UserError", "ValidationError") | |
| assert not semantics._is_subclass_of("RedirectWarning", "UserError") | |
| # ---------------------------------------------- triggers on the two conditionals | |
| def facts(declared=(), deleted=(), multi_company=False, complete=True): | |
| return {"walked": True, "complete": complete, "files": 1, | |
| "models_declared": set(declared), "models_deleted": set(deleted), | |
| "multi_company": multi_company} | |
| UNGUARDED = ONDELETE_BODY.format(body=" return self.model_id.model") | |
| def test_ondelete_says_nothing_when_the_parent_is_never_deleted(): | |
| """Master data. The rule's harm is a read through a NULLED reference, and | |
| nothing nulls one if nothing deletes the parent.""" | |
| quiet = facts(declared=["ir.model"]) | |
| assert semantics.run_check("ondelete_leaves_a_dangling_read", | |
| UNGUARDED, quiet) == [] | |
| def test_ondelete_is_a_defect_when_the_parent_really_is_deleted(): | |
| found = semantics.run_check("ondelete_leaves_a_dangling_read", UNGUARDED, | |
| facts(declared=["ir.model"], | |
| deleted=["ir.model"])) | |
| assert len(found) == 1 | |
| assert found[0][2] == "defect", "a trigger that fired should promote" | |
| assert "does delete ir.model" in found[0][1] | |
| def test_ondelete_keeps_its_grade_when_the_trigger_cannot_be_answered(): | |
| """No tree, or a parent this tree does not define. The rule is reported | |
| exactly as it was before triggers existed, which is the honest answer.""" | |
| for supplied in (None, facts(declared=["something.else"])): | |
| found = semantics.run_check("ondelete_leaves_a_dangling_read", | |
| UNGUARDED, supplied) | |
| assert len(found) == 1 | |
| assert found[0][2] is None, "an unanswered trigger must not promote" | |
| CHECK_COMPANY_BODY = ''' | |
| class Thing(models.Model): | |
| _name = "my.thing" | |
| company_id = fields.Many2one("res.company") | |
| journal_id = fields.Many2one("account.journal") | |
| ''' | |
| def test_check_company_says_nothing_in_a_single_company_codebase(): | |
| """One company cannot leak to another. The rule describes a boundary this | |
| codebase does not have.""" | |
| assert semantics.run_check("check_company_missing_on_company_owned_relation", | |
| CHECK_COMPANY_BODY, facts()) == [] | |
| def test_check_company_is_a_defect_where_multi_company_is_in_use(): | |
| found = semantics.run_check("check_company_missing_on_company_owned_relation", | |
| CHECK_COMPANY_BODY, facts(multi_company=True)) | |
| assert len(found) == 1 | |
| assert found[0][2] == "defect" | |
| assert "runs multi-company" in found[0][1] | |
| def test_check_company_keeps_its_grade_with_no_tree(): | |
| found = semantics.run_check("check_company_missing_on_company_owned_relation", | |
| CHECK_COMPANY_BODY, None) | |
| assert len(found) == 1 and found[0][2] is None | |
| def test_an_incomplete_walk_never_silences_either_rule(): | |
| """Promote on evidence, never drop on the absence of it. A module-sized walk | |
| is exactly the case where absence means 'not in this module'.""" | |
| partial = facts(declared=["ir.model"], complete=False) | |
| assert len(semantics.run_check("ondelete_leaves_a_dangling_read", | |
| UNGUARDED, partial)) == 1 | |
| assert len(semantics.run_check("check_company_missing_on_company_owned_relation", | |
| CHECK_COMPANY_BODY, partial)) == 1 | |
| # ------------------------------------------ depends: prefix coverage, both ways | |
| STORED_COMPUTE_DOTTED = """total = fields.Float(compute='_c', store=True) | |
| @api.depends('line_ids.price') | |
| def _c(self): | |
| for rec in self: | |
| rec.total = sum(l.price for l in rec.line_ids)""" | |
| STORED_COMPUTE_ROOT_ONLY = """total = fields.Float(compute='_c', store=True) | |
| @api.depends('line_ids') | |
| def _c(self): | |
| for rec in self: | |
| rec.total = sum(l.price for l in rec.line_ids)""" | |
| def test_a_dotted_depends_covers_a_read_of_its_own_prefix(): | |
| """@api.depends('line_ids.price') registers the whole chain, so the ORM | |
| already recomputes when line_ids itself changes and a body reading | |
| rec.line_ids needs nothing more. | |
| Comparing declarations as exact strings made that correct code stale: 42 of | |
| this checker's 104 findings on OpenSPP were this, 40% of its output. The | |
| rule's own `correct` sample hid it by declaring 'line_ids' redundantly | |
| alongside the dotted paths, so validate passed throughout. | |
| """ | |
| assert semantics.depends_covers_compute_reads(STORED_COMPUTE_DOTTED) == [] | |
| def test_a_declared_prefix_still_does_not_cover_what_hangs_off_it(): | |
| """The converse, which is the rule itself and must not follow. | |
| Declaring 'line_ids' cannot excuse a body reading line_ids.price -- that is | |
| the value that goes stale, and the whole reason the checker exists. A | |
| coverage test asserting only the first direction would happily accept a | |
| checker that had stopped working. | |
| """ | |
| hits = semantics.depends_covers_compute_reads(STORED_COMPUTE_ROOT_ONLY) | |
| assert hits, "the incomplete depends stopped being reported" | |
| assert any("line_ids.price" in message for _, message in hits) | |
| # -------------------------------------- decorators nothing can read statically | |
| COMPUTED_CONSTRAINS = """@api.constrains(*FIELDS) | |
| def _check(self): | |
| for rec in self: | |
| if rec.date_start > rec.date_end: | |
| raise ValidationError('bad range')""" | |
| def test_a_computed_decorator_is_a_gap_and_not_a_finding(): | |
| """@api.constrains(*FIELDS) is legal code that a static pass cannot read. | |
| It used to collapse into the same answer as "declares nothing", so every | |
| field the body read became a missing declaration -- and the sentence it | |
| produced said "declares only nothing", which is where the bug announced | |
| itself. Nothing is wrong with the code, so nothing is reported; what could | |
| not be decided is recorded instead. | |
| """ | |
| gaps = [] | |
| assert semantics.constrains_lists_every_field_read( | |
| COMPUTED_CONSTRAINS, gaps=gaps) == [] | |
| assert gaps and "not string literals" in gaps[0] | |
| assert "_check()" in gaps[0], "the gap names the method it could not read" | |
| def test_every_checker_survives_a_decorator_it_cannot_read(): | |
| """Four checkers read a decorator's arguments and two of them only tested | |
| for absence. Handing them a sentinel they did not expect turned a false | |
| positive into a crash, which scan_source reports as a broken checker.""" | |
| for name in semantics.CHECKS: | |
| semantics.run_check(name, COMPUTED_CONSTRAINS) | |
| # ------------------------------------------ constrains_declares_a_dotted_name | |
| def dotted(code, gaps=None): | |
| return semantics.constrains_declares_a_dotted_name(code, gaps=gaps) | |
| def test_a_dotted_name_of_three_segments_is_caught(): | |
| """The regex wanted a closing quote after the second segment. In | |
| partner_id.country_id.code it found a dot there, and said nothing about a | |
| name the ORM ignores exactly as it ignores a two-segment one.""" | |
| hits = dotted("""@api.constrains('partner_id.country_id.code') | |
| def _check_country(self): | |
| for rec in self: | |
| if rec.partner_id.country_id.code not in ('BE', 'FR'): | |
| raise ValidationError('Unsupported country')""") | |
| assert [line for line, _ in hits] == [1] | |
| assert "'partner_id.country_id.code'" in hits[0][1] | |
| assert "never runs the check" in hits[0][1] | |
| def test_a_dotted_name_with_a_digit_in_it_is_caught(): | |
| """[a-z_] has no digits, so every name through an l10n_ field was invisible | |
| to the regex. The plain name beside it still registers, and the message says | |
| that is now the only write that runs the check.""" | |
| hits = dotted("""@api.constrains('l10n_in_gst_treatment', 'partner_id.l10n_in_pan') | |
| def _check_pan(self): | |
| for move in self: | |
| if not move.partner_id.l10n_in_pan: | |
| raise ValidationError('PAN required')""") | |
| assert len(hits) == 1 | |
| assert "'partner_id.l10n_in_pan'" in hits[0][1] | |
| assert "only when l10n_in_gst_treatment is written" in hits[0][1] | |
| def test_plain_names_are_not_a_finding(): | |
| """Every literal declaration on the three trees is this shape, 41 of them | |
| with a digit in a name. A checker that fired here would fire on all of it.""" | |
| assert dotted("""@api.constrains('l10n_in_pan', 'date_start', 'date_end') | |
| def _check(self): | |
| pass""") == [] | |
| assert dotted("""@api.constrains() | |
| def _check(self): | |
| pass""") == [] | |
| def test_a_dotted_name_is_reported_on_its_decorator(): | |
| """Where the names are and where a marker goes, whichever line the dotted | |
| one sits on and whatever else decorates the method.""" | |
| hits = dotted("""class Move(models.Model): | |
| @api.model | |
| @api.constrains( | |
| 'name', | |
| 'partner_id.country_id', | |
| ) | |
| def _check(self): | |
| pass""") | |
| assert [line for line, _ in hits] == [3] | |
| def test_a_computed_constrains_list_is_a_gap_and_not_a_finding(): | |
| """*FIELDS might hold a dotted name or not; nothing here can say. Reading it | |
| as "nothing dotted" would be a clean result nobody earned.""" | |
| gaps = [] | |
| assert dotted(COMPUTED_CONSTRAINS, gaps=gaps) == [] | |
| assert gaps and "not string literals" in gaps[0] and "_check()" in gaps[0] | |
| def test_a_method_neither_constrains_checker_can_read_is_one_gap(): | |
| """Both constrains checkers skip @api.constrains(*FIELDS), and each says so. | |
| `check` printed that twice, and fieldtest counted it twice as a file it | |
| could not fully read.""" | |
| import layers | |
| assert {"constrains_lists_every_field_read", | |
| "constrains_declares_a_dotted_name"} <= semantics.GAP_CHECKS | |
| rules = [r for r in corpus.load_rules() if r["stack"] == "odoo"] | |
| gaps = [] | |
| layers.scan_source(COMPUTED_CONSTRAINS, rules, "odoo", gaps=gaps) | |
| assert len(gaps) == 1, gaps | |
| # ---------------------------------------------- balance_js: regex vs division | |
| def test_a_regex_after_a_keyword_is_not_read_as_division(): | |
| """`return /[(]/` is ordinary JavaScript. The heuristic only knew about | |
| punctuation, so the slash after a keyword read as division and the regex | |
| BODY was scanned as code -- its lone bracket reported as unbalanced in a | |
| file that is fine. That reaches the user as a high finding, and in | |
| `nexa run` it buys an escalation to the 30B over nothing at all.""" | |
| for source in ("function f(){ return /[(]/.test(s); }", | |
| "switch (c) { case 'a': return /]/.test(v); }", | |
| "const ok = typeof v === 'string' && /[)]/.test(v);", | |
| "function g(){ return /[{]/.test(x) ? 1 : 2; }"): | |
| assert semantics.balance_js(source) == [], source | |
| def test_division_stays_division_and_a_real_imbalance_still_fires(): | |
| """The other half of the same change. Widening the regex heuristic must not | |
| turn arithmetic into a literal, and must not stop the scanner catching the | |
| truncated answer it exists for.""" | |
| assert semantics.balance_js("const r = total / count; f();") == [] | |
| assert semantics.balance_js("const r = 10 / 2; f();") == [] | |
| assert semantics.balance_js("function f(){ return 1;") != [] | |
| assert semantics.balance_js("function f(){ return [1; }") != [] | |
| # ------------------------------- narrowings from the OCA classification (3.43) | |
| WIZARD_STORED_COMPUTE = """class Wiz(models.TransientModel): | |
| _name = "my.wizard" | |
| total = fields.Float(compute="_c", store=True) | |
| @api.depends("line_ids") | |
| def _c(self): | |
| for rec in self: | |
| rec.total = sum(l.price for l in rec.line_ids) | |
| """ | |
| ONDELETE_SHAPE = """class Alert(models.{kind}): | |
| _name = "spp.alert" | |
| model_id = fields.Many2one("ir.model"{extra}) | |
| def run(self): | |
| return self.model_id.model | |
| """ | |
| CHECK_COMPANY_SHAPE = """class Thing(models.Model): | |
| _name = "my.thing" | |
| company_id = fields.Many2one("res.company", required=True) | |
| journal_id = fields.Many2one("account.journal"{extra}) | |
| """ | |
| MULTI_COMPANY = {"walked": True, "complete": True, "files": 1, | |
| "models_declared": set(), "models_deleted": set(), | |
| "multi_company": True} | |
| def test_a_wizard_is_not_asked_for_a_complete_depends(): | |
| """A TransientModel row is vacuumed. "Written once and never recomputed, | |
| survives restarts, diverges from reality" is about a row that outlives what | |
| it was computed from, and a wizard's does not. Five of the 24 false | |
| positives classified on OCA were this.""" | |
| assert semantics.depends_covers_compute_reads(WIZARD_STORED_COMPUTE) == [] | |
| def test_the_same_compute_on_a_persistent_model_still_fires(): | |
| """The narrowing has to cost nothing. Identical code on a models.Model is | |
| the defect the rule exists for.""" | |
| persistent = WIZARD_STORED_COMPUTE.replace("models.TransientModel", "models.Model") | |
| assert semantics.depends_covers_compute_reads(persistent) | |
| def test_ondelete_is_not_asked_of_a_wizard_or_a_field_with_no_column(): | |
| """ondelete governs a foreign key. A TransientModel's row is vacuumed before | |
| a parent can be deleted under it, and a compute= without store=True has no | |
| column at all -- so in both the advice names something that is not there.""" | |
| assert semantics.ondelete_leaves_a_dangling_read( | |
| ONDELETE_SHAPE.format(kind="TransientModel", extra="")) == [] | |
| assert semantics.ondelete_leaves_a_dangling_read( | |
| ONDELETE_SHAPE.format(kind="Model", extra=', compute="_c"')) == [] | |
| def test_ondelete_still_fires_on_a_real_column(): | |
| """Both halves of the same predicate: a persistent model, and a compute that | |
| IS stored, still have a foreign key to govern.""" | |
| assert semantics.ondelete_leaves_a_dangling_read( | |
| ONDELETE_SHAPE.format(kind="Model", extra="")) | |
| assert semantics.ondelete_leaves_a_dangling_read( | |
| ONDELETE_SHAPE.format(kind="Model", extra=', compute="_c", store=True')) | |
| def test_check_company_is_not_asked_of_a_mirror_or_a_field_with_no_column(): | |
| """A related= field mirrors a value owned by another record, so the | |
| constraint belongs on the source -- which the same run flags in its own | |
| right, making this the same decision reported twice. A computed field with | |
| no store has no column to constrain.""" | |
| assert semantics.check_company_missing_on_company_owned_relation( | |
| CHECK_COMPANY_SHAPE.format(extra=', related="parent_id.journal_id"'), | |
| MULTI_COMPANY) == [] | |
| assert semantics.check_company_missing_on_company_owned_relation( | |
| CHECK_COMPANY_SHAPE.format(extra=', compute="_c"'), MULTI_COMPANY) == [] | |
| def test_check_company_still_fires_on_a_stored_relation(): | |
| assert semantics.check_company_missing_on_company_owned_relation( | |
| CHECK_COMPANY_SHAPE.format(extra=""), MULTI_COMPANY) | |
| assert semantics.check_company_missing_on_company_owned_relation( | |
| CHECK_COMPANY_SHAPE.format(extra=', compute="_c", store=True'), MULTI_COMPANY) | |
| MESSAGE_IN_A_VARIABLE = """@api.constrains("sequence_id") | |
| def _check(self): | |
| for j in self: | |
| if j.sequence_id and not j.sequence_id.company_id: | |
| msg = _("no company on %(s)s for %(j)s", s=j.sequence_id.name, j=j.display_name) | |
| raise ValidationError(msg) | |
| """ | |
| def test_a_message_built_into_a_variable_is_still_only_a_message(): | |
| """A field read to phrase an error does not decide whether the invariant | |
| holds, so it cannot decide when the constraint must re-run. That exemption | |
| existed and knew only the inline spelling -- OCA builds the message one line | |
| before the raise, and display_name was reported as a dependency.""" | |
| assert semantics.constrains_lists_every_field_read(MESSAGE_IN_A_VARIABLE) == [] | |
| def test_a_field_read_in_the_condition_is_still_caught(): | |
| """The exemption is about the nodes inside the message, not the field name. | |
| A constraint that TESTS an undeclared field is the defect either way.""" | |
| hits = semantics.constrains_lists_every_field_read( | |
| MESSAGE_IN_A_VARIABLE.replace("not j.sequence_id.company_id", | |
| "not j.active")) | |
| assert any("active" in message for _, message in hits) | |
| CALLERS_CURSOR = """def go(self): | |
| for rec in self: | |
| rec.write({"x": 1}) | |
| self.env.cr.commit() | |
| """ | |
| OWN_CURSOR = """def batch_unlink(self): | |
| with Registry(self.env.cr.dbname).cursor() as new_cr: | |
| new_env = api.Environment(new_cr, self.env.uid, self.env.context) | |
| while self: | |
| batch = self[0:1000] | |
| self -= batch | |
| batch.with_env(new_env).unlink() | |
| new_env.cr.commit() | |
| """ | |
| def test_a_commit_on_a_cursor_this_function_opened_is_not_the_callers(): | |
| """The rule says let the CALLER own the transaction, and a cursor opened | |
| three lines earlier is not the caller's. Both of this rule's real-world | |
| instances are the two sides of that line: OpenSPP's expiry cron commits | |
| self.env.cr and is a true positive its author marked and kept, OCA's | |
| batch_unlink commits one it opened itself.""" | |
| assert semantics.commit_inside_a_loop(CALLERS_CURSOR) | |
| assert semantics.commit_inside_a_loop(OWN_CURSOR) == [] | |
| def test_the_independent_cursor_exemption_does_not_leak_between_functions(): | |
| """Bound positively and scoped per function. A new_cr opened in one method | |
| must not excuse a commit in another that reuses the name -- which is what a | |
| file-wide scan of the names would have done.""" | |
| hits = semantics.commit_inside_a_loop(CALLERS_CURSOR + OWN_CURSOR) | |
| assert len(hits) == 1, hits | |
| # ------------------------------ a constraint the series will not create (3.45) | |
| UNIQUENESS_BESIDE = """class Thing(models.Model): | |
| _name = "my.thing" | |
| {declaration} | |
| @api.constrains("code") | |
| def _check_unique_code(self): | |
| for rec in self: | |
| if self.search_count([("code", "=", rec.code), ("id", "!=", rec.id)]): | |
| raise ValidationError("Code must be unique") | |
| """ | |
| LEGACY = '_sql_constraints = [("code_uniq", "UNIQUE(code)", "Code must be unique.")]' | |
| MODERN = '_code_uniq = models.Constraint("UNIQUE(code)", "Code must be unique.")' | |
| def test_a_legacy_sql_constraint_is_not_a_constraint_on_odoo_19(): | |
| """Odoo 19 ignores the _sql_constraints attribute: the registry logs a | |
| warning at load and the constraint is never created. Reading it as the | |
| column being spoken for exempts a class on the strength of nothing, and | |
| leaves the racing Python check as the only thing standing.""" | |
| code = UNIQUENESS_BESIDE.format(declaration=LEGACY) | |
| assert semantics.prefer_sql_constraint_applies(code, {"odoo_series": "19.0"}) | |
| def test_the_same_declaration_still_counts_on_16(): | |
| """It is a real constraint there, and firing on every 16, 17 and 18 module | |
| that did exactly the right thing is the worse trade.""" | |
| code = UNIQUENESS_BESIDE.format(declaration=LEGACY) | |
| assert semantics.prefer_sql_constraint_applies(code, {"odoo_series": "16.0"}) == [] | |
| assert semantics.prefer_sql_constraint_applies(code, {"odoo_series": None}) == [] | |
| assert semantics.prefer_sql_constraint_applies(code) == [] | |
| def test_models_constraint_counts_on_every_series(): | |
| """The spelling that works from 17 onwards is never the dead one.""" | |
| code = UNIQUENESS_BESIDE.format(declaration=MODERN) | |
| for series in (None, "16.0", "17.0", "19.0"): | |
| assert semantics.prefer_sql_constraint_applies( | |
| code, {"odoo_series": series}) == [], series | |
| def test_the_corpus_does_not_advise_a_declaration_odoo_19_ignores(): | |
| """Two rules told an author to write _sql_constraints, which from 19 creates | |
| nothing. A corpus whose subject is 'a check that looks like it holds and | |
| does not' cannot be the thing recommending one.""" | |
| advising = [] | |
| for rule in corpus.load_rules(): | |
| for line in rule["correct"].splitlines(): | |
| # In a comment it is documentation of the older spelling, which is | |
| # the point. Uncommented, it is the advice. | |
| if "_sql_constraints" in line and not line.lstrip().startswith("#"): | |
| advising.append(rule["id"]) | |
| assert advising == [], advising | |
| def test_the_onchange_guard_knows_every_spelling_of_a_constraint(): | |
| """The guard has to accept the legacy attribute: that IS a constraint on 16 | |
| through 18. Narrowing it to models.Constraint alone was tried and reverted -- | |
| the next field test reported OCA's account_loan, branch 18.0, with a real | |
| _sql_constraints block and an onchange properly backed by it. | |
| The 19 case is closed where the series is legible instead, in | |
| prefer_sql_constraint_applies. | |
| Asserted against the checker rather than against a satisfied_pattern, | |
| which is where the guard moved when the rule became an AST pass. The | |
| three spellings are the same three.""" | |
| body = ('class M(models.Model):\n' | |
| ' @api.onchange("qty")\n' | |
| ' def _onchange_qty(self):\n' | |
| ' if self.qty <= 0:\n' | |
| ' raise ValidationError("no")\n') | |
| assert semantics.run_check("onchange_tries_to_enforce", body), \ | |
| "an unbacked onchange that raises is the violation" | |
| for spelling in (' @api.constrains("qty")\n def _check(self):\n pass\n', | |
| ' _sql_constraints = [("q", "CHECK(qty > 0)", "no")]\n', | |
| ' _qty = models.Constraint("CHECK(qty > 0)", "no")\n'): | |
| backed = body + spelling | |
| assert not semantics.run_check("onchange_tries_to_enforce", backed), spelling | |
| def test_a_dead_sql_constraint_does_not_back_an_onchange_on_19(): | |
| """From 19 the registry never creates a _sql_constraints block, so it | |
| backs nothing. On 18 and on an unknown series it still counts, as the | |
| test above requires.""" | |
| body = ('class M(models.Model):\n' | |
| ' _sql_constraints = [("q", "CHECK(qty > 0)", "no")]\n' | |
| '\n' | |
| ' @api.onchange("qty")\n' | |
| ' def _onchange_qty(self):\n' | |
| ' if self.qty <= 0:\n' | |
| ' raise ValidationError("no")\n') | |
| on_19 = semantics.run_check("onchange_tries_to_enforce", body, | |
| {"odoo_series": "19.0"}) | |
| assert [line for line, _, _ in on_19] == [4], on_19 | |
| for series in ("18.0", None): | |
| assert semantics.run_check("onchange_tries_to_enforce", body, | |
| {"odoo_series": series}) == [], series | |
| real = body.replace(' _sql_constraints = [("q", "CHECK(qty > 0)", "no")]\n', | |
| ' _q = models.Constraint("CHECK(qty > 0)", "no")\n') | |
| assert semantics.run_check("onchange_tries_to_enforce", real, | |
| {"odoo_series": "19.0"}) == [] | |
| CREDIT_ONCHANGE = ('class M(models.Model):\n' | |
| '{constraint}' | |
| ' @api.onchange("credit_limit")\n' | |
| ' def _onchange_credit_limit(self):\n' | |
| ' if self.credit_limit < 0:\n' | |
| ' raise ValidationError("no")\n') | |
| def test_a_constraint_on_another_field_does_not_back_an_onchange(constraint): | |
| """A uniqueness rule on `reference` used to silence an onchange raising | |
| on `credit_limit`. A dotted @api.constrains name registers nothing.""" | |
| code = CREDIT_ONCHANGE.format(constraint=constraint) | |
| assert semantics.run_check("onchange_tries_to_enforce", code), constraint | |
| def test_a_constraint_on_a_watched_field_or_an_unreadable_one_backs_it(constraint): | |
| code = CREDIT_ONCHANGE.format(constraint=constraint) | |
| assert semantics.run_check("onchange_tries_to_enforce", code) == [], constraint | |
| WARNING_ONCHANGE = ('class M(models.Model):\n' | |
| ' @api.onchange("qty")\n' | |
| ' def _onchange_qty(self):\n' | |
| ' self.hint = False\n' | |
| ' if self.qty < 0:\n' | |
| '{branch}' | |
| ' return {{"warning": {{"title": "x", "message": "y"}}}}\n') | |
| def test_a_warning_refuses_only_when_its_branch_takes_the_value_back(branch, refuses): | |
| """Most warning dicts on core inform: "this reference already exists", | |
| "the rate is far from the previous one". The user saves past them in the | |
| form as in an import, so nothing is enforced anywhere. `self.hint = False` | |
| outside the branch does not count: it is a UI flag, not the refusal.""" | |
| code = WARNING_ONCHANGE.format(branch=branch) | |
| found = semantics.run_check("onchange_tries_to_enforce", code) | |
| assert bool(found) is refuses, (branch, found) | |
| UNPLAN = ('_("It is not possible to unplan one single Work Order. "\n' | |
| ' "Unplan the manufacturing order instead.")') | |
| WORKORDER = ('class W(models.Model):\n' | |
| ' _name = "w"\n' | |
| ' date_finished = fields.Datetime({field})\n' | |
| '\n' | |
| ' def {server}(self):\n' | |
| ' if not self.date_finished:\n' | |
| ' raise UserError({server_message})\n' | |
| '\n' | |
| ' @api.onchange("date_finished")\n' | |
| ' def _onchange_date_finished(self):\n' | |
| ' if not self.date_finished:\n' | |
| ' raise UserError(' + UNPLAN + ')\n') | |
| def test_the_server_raising_the_same_message_backs_an_onchange(field, server, | |
| message, backed): | |
| """The same sentence raised again where the server runs it on every | |
| write is the same check. A paraphrase is not, and neither is a method | |
| the client has to choose to call.""" | |
| code = WORKORDER.format(field=field, server=server, server_message=message) | |
| found = semantics.run_check("onchange_tries_to_enforce", code) | |
| assert (found == []) is backed, found | |
| APIKEY = ('class D(models.TransientModel):\n' | |
| ' _name = "d"\n' | |
| '\n' | |
| ' @api.onchange("expiration_date")\n' | |
| ' def _onchange_expiration_date(self):\n' | |
| ' self.env["k"]._check_expiration_date({onchange_arg})\n' | |
| ' if not self.expiration_date:\n' | |
| ' raise UserError("no")\n' | |
| '\n' | |
| ' def create(self, vals_list):\n' | |
| ' res = super().create(vals_list)\n' | |
| ' self.env["k"]._check_expiration_date(res.expiration_date)\n' | |
| ' return res\n') | |
| def test_the_same_check_helper_on_the_watched_field_backs_an_onchange(): | |
| """res.users.apikeys.description: the onchange and create() both call | |
| _check_expiration_date on the watched field. A check helper handed | |
| nothing the onchange watches, like a bare _check_company(), is not.""" | |
| backed = APIKEY.format(onchange_arg="self.expiration_date") | |
| assert semantics.run_check("onchange_tries_to_enforce", backed) == [] | |
| other = APIKEY.format(onchange_arg="self.company_id") | |
| assert semantics.run_check("onchange_tries_to_enforce", other) | |
| # ------------------------------------------------ the value-origin trace | |
| # | |
| # Four rules were classified false for one reason: each read a variable's NAME | |
| # and treated that as evidence about its value. Every case below is a finding | |
| # somebody read by hand and recorded a verdict for, in | |
| # artifacts/classification-{oca,openspp,python}-2026-09.json. | |
| INT_ZERO_CASES = [ | |
| ("search_count", """ | |
| def f(self): | |
| total_population = self.env["res.partner"].search_count([]) | |
| if total_population == 0: | |
| return 0 | |
| """), | |
| ("read_group _count, keyed by an f-string", """ | |
| def f(self, field_name, rows): | |
| for row in rows: | |
| group_total = row[f"{field_name}_count"] | |
| if group_total == 0: | |
| continue | |
| """), | |
| ("SELECT COUNT(*) through a cursor", """ | |
| def f(cr, table, where): | |
| cr.execute(f"SELECT COUNT(*) FROM {table} WHERE {where}") | |
| total = cr.fetchone()[0] | |
| if total == 0: | |
| return [] | |
| """), | |
| ("a field the same file declares as fields.Integer", """ | |
| class L(models.Model): | |
| qty = fields.Integer(string="Quantity") | |
| def f(self): | |
| self.line_ids.filtered(lambda x: x.qty == 0).unlink() | |
| """), | |
| ] | |
| def test_an_integer_compared_to_zero_is_not_a_float_comparison(label, body): | |
| """All five false positives on this rule compared an INTEGER to zero, where | |
| == is exact and the rule has nothing to say. Three of them and both true | |
| positives were spelled `total == 0`, so the name could never have separated | |
| them.""" | |
| assert semantics.run_check("float_equality_against_zero", body) == [], label | |
| def test_a_float_compared_to_zero_still_fires(): | |
| """The guard suppresses; it must not suppress everything. `sum()` over | |
| amounts is the true positive that shares its spelling with three of the | |
| false ones.""" | |
| body = """ | |
| def f(sorted_amounts): | |
| total = sum(sorted_amounts) | |
| if total == 0: | |
| return 0.0 | |
| """ | |
| assert semantics.run_check("float_equality_against_zero", body) | |
| def test_a_locally_built_filename_is_not_a_traversal(): | |
| """0 true, 4 false. All four join a configured directory to a name the | |
| module itself produced, and the pattern matched the word `filename`.""" | |
| body = """ | |
| def f(self, data_path): | |
| for file_config in [{"filename": "a.csv"}]: | |
| file_path = os.path.join(data_path, file_config["filename"]) | |
| """ | |
| assert semantics.run_check("path_join_takes_an_external_name", body) == [] | |
| def test_a_component_from_outside_the_file_still_fires(): | |
| body = "full = os.path.join(UPLOAD_DIR, request.args['name'])\n" | |
| assert semantics.run_check("path_join_takes_an_external_name", body) | |
| def test_resolving_the_path_clears_the_join_in_that_function_only(): | |
| """The fix is to resolve and compare, and it is written beside the join. A | |
| realpath in a DIFFERENT function says nothing about this one.""" | |
| fixed = """ | |
| def f(base): | |
| full = os.path.realpath(os.path.join(base, request.args['name'])) | |
| if os.path.commonpath([base, full]) != base: | |
| raise ValueError("escapes") | |
| """ | |
| assert semantics.run_check("path_join_takes_an_external_name", fixed) == [] | |
| elsewhere = fixed + """ | |
| def g(base): | |
| return open(os.path.join(base, request.args['other']), 'rb') | |
| """ | |
| assert semantics.run_check("path_join_takes_an_external_name", elsewhere) | |
| def test_the_orm_monetary_idiom_is_not_a_decimal_violation(): | |
| """Odoo's Monetary field IS a float -- the ORM defines it that way -- so | |
| inside a module the rule fired on the idiom and named a remedy that cannot | |
| be taken. Both shapes below were classified false.""" | |
| for body in (""" | |
| class E(models.Model): | |
| def f(self): | |
| ctx = {"base_amount": float(self.amount or 0.0)} | |
| """, """ | |
| def f(self): | |
| result = self.env["spp.cel.service"].evaluate_expression(self.e, {}) | |
| amount = float(result) | |
| """): | |
| assert semantics.run_check("float_coercion_of_external_money", body) == [], body | |
| def test_a_money_string_from_outside_still_fires(): | |
| """Where the value arrives as a string, Decimal(value) IS available and the | |
| rule's remedy can be taken. This is the rule's own wrong sample.""" | |
| body = "price = float(payload['price'])\n" | |
| assert semantics.run_check("float_coercion_of_external_money", body) | |
| def test_an_onchange_does_not_answer_for_the_method_below_it(): | |
| """All three false positives on this rule were the same failure: a short | |
| onchange that assigns a field, followed by an action method that raises, | |
| with the regex's 400-character window crossing between them. | |
| spp_studio/wizard/variable_remap_wizard.py is the shape, verbatim.""" | |
| body = """ | |
| class W(models.TransientModel): | |
| @api.onchange("variable_id") | |
| def _onchange_variable_id(self): | |
| if self.variable_id and self.variable_id.source_model: | |
| self.target_model = self.variable_id.source_model | |
| def action_remap(self): | |
| self.ensure_one() | |
| if not self.new_field_id: | |
| raise UserError(_("Please select a field to map to.")) | |
| """ | |
| assert semantics.run_check("onchange_tries_to_enforce", body) == [] | |
| def test_the_onchange_finding_anchors_on_the_decorator(): | |
| """Where the rule is, and where a marker about it gets written. A | |
| FunctionDef's own lineno is the `def`, one line further down.""" | |
| body = ('class M(models.Model):\n' | |
| ' @api.onchange("qty")\n' | |
| ' def _onchange_qty(self):\n' | |
| ' raise ValidationError("no")\n') | |
| found = semantics.run_check("onchange_tries_to_enforce", body) | |
| assert [line for line, _, _ in found] == [2] | |
| # ------------------------------------------------- sudo, all three clauses | |
| def test_sudo_covers_the_three_clauses_its_rule_states(clause, body): | |
| """The rule says three things -- scope it narrowly, re-apply company | |
| filtering, never pass a sudo recordset onward -- and only the read was | |
| checked. `.sudo(` is the largest cluster in the recall harvest at 70 | |
| missed against 6 caught, and on the OpenSPP tree the unchecked write | |
| clause has 136 call sites against the read clause's 116.""" | |
| assert semantics.run_check("sudo_drops_record_rules", body), clause | |
| def test_sudo_stays_quiet_where_the_rule_is_already_followed(why, body): | |
| assert semantics.run_check("sudo_drops_record_rules", body) == [], why | |
| # ------------------------------------------------------------- docstrings | |
| def test_a_docstring_is_prose_and_is_not_matched_as_code(): | |
| """Two of the 50 findings the Python composition added were this: `eval()` | |
| named in a docstring in a sentence arguing against using it, and a | |
| `datetime.now()` inside a doctest example.""" | |
| src = ('def f():\n' | |
| ' """Never use eval() here.\n' | |
| '\n' | |
| ' >>> datetime.now()\n' | |
| ' """\n' | |
| ' return 1\n') | |
| view = semantics.strip_comments(src) | |
| assert "eval(" not in view | |
| assert "datetime.now()" not in view | |
| def test_a_real_string_literal_survives_and_the_geometry_does_too(): | |
| """Only docstrings. A triple-quoted SQL query is data the checkers have a | |
| legitimate interest in, and a line or column that moved would misreport | |
| every finding below it.""" | |
| src = 'Q = """SELECT COUNT(*) FROM t"""\n\ndef f():\n """doc"""\n return Q\n' | |
| view = semantics.strip_comments(src) | |
| assert "SELECT COUNT(*)" in view | |
| assert "doc" not in view | |
| assert len(view) == len(src) | |
| assert view.count("\n") == src.count("\n") | |
| def test_rebinding_self_to_sudo_elevates_the_rest_of_the_method(): | |
| """The shape behind the one miss the README names by hand. | |
| OCA server-tools cbefaa511485, "[FIX] excel_import_export: remove sudo() | |
| when importing record", deletes a single line -- `self = self.sudo()` -- | |
| and says what it cost: "finding multiple records across all companies | |
| during import when they have same record name. Previously, importing with | |
| sudo could bypass multi-company record rules." | |
| Nothing below that line names sudo, so every clause that looks for | |
| `.sudo().<method>()` is blind to it, and the recordset is never passed | |
| anywhere either. It replaces the thing the whole method is written | |
| against, which is the escape clause at its widest. | |
| Run against that commit's own before and after images, the finding sits on | |
| line 283, is gone afterwards, and 283 is one of the two lines the fix | |
| changed -- an on-target catch by recall's own definition. | |
| """ | |
| before = ("class X(models.AbstractModel):\n" | |
| " def import_xlsx(self, res_id, template):\n" | |
| " self = self.sudo()\n" | |
| " record = self.env[template.res_model].browse(res_id)\n" | |
| " return record\n") | |
| after = before.replace(" self = self.sudo()\n", "") | |
| found = semantics.run_check("sudo_drops_record_rules", before) | |
| assert [line for line, _, _ in found] == [3] | |
| assert not semantics.run_check("sudo_drops_record_rules", after) | |
| def test_a_rebind_keeps_the_rules_own_grade(): | |
| """The touch and escape clauses report `unproven` because nothing measured | |
| has met a real one. This clause has: a fix commit that deletes the line and | |
| explains the multi-company leak it caused. So it carries no grade of its | |
| own and keeps the rule's `defect`.""" | |
| body = ("def f(self):\n" | |
| " self = self.sudo()\n" | |
| " return self.env['res.partner'].search([])\n") | |
| grades = {grade for _, _, grade in | |
| semantics.run_check("sudo_drops_record_rules", body)} | |
| assert None in grades, "a rebind must not be downgraded to unproven" | |
| def test_a_sudo_recordset_bound_to_a_fresh_name_is_not_a_rebind(): | |
| """`p = self.env['x'].sudo()` binds a new name and elevates nothing that | |
| was not already written against it. Only rebinding a name to its OWN sudo | |
| form silently elevates the code below.""" | |
| body = ("def f(self):\n" | |
| " p = self.env['res.partner'].sudo()\n" | |
| " return p.mapped('name')\n") | |
| assert semantics.run_check("sudo_drops_record_rules", body) == [] | |
| def test_sudo_false_is_de_escalation_and_is_never_a_finding(): | |
| """`sudo(False)` turns record rules back ON. | |
| It is how a method that must run under the caller's own rights says so. | |
| Odoo core does it 60 times, including mail's `_check_attachments_access`, | |
| whose docstring reads "This method relies on access rules/rights and | |
| therefore it should not be called from a sudo env." Reading it as an | |
| escalation reports the safeguard as the defect. | |
| Neither OpenSPP nor the OCA trees contain a single `sudo(False)`, so this | |
| was invisible until the corpus was pointed at odoo core. | |
| """ | |
| for body in ("class A(models.Model):\n" | |
| " def f(self, toks):\n" | |
| " self = self.sudo(False)\n" | |
| " return self.env['x'].search([])\n", | |
| "def f(self):\n" | |
| " return self.env['res.partner'].sudo(False).search([])\n", | |
| "def f(self, vals):\n" | |
| " return self.env['res.partner'].sudo(False).create(vals)\n"): | |
| assert semantics.run_check("sudo_drops_record_rules", body) == [], body | |
| def test_a_computed_sudo_flag_still_counts_as_escalation(): | |
| """`sudo(flag)` can be either, and an unreadable argument must leave the | |
| finding standing rather than suppress it -- the same direction every other | |
| trigger on this rule errs in.""" | |
| body = ("def f(self, flag):\n" | |
| " self = self.sudo(flag)\n" | |
| " return self.env['x'].search([])\n") | |
| assert semantics.run_check("sudo_drops_record_rules", body) | |
| def test_an_unstored_framework_field_is_not_a_stored_compute(): | |
| """odoo/models.py adds display_name to every model as | |
| `fields.Char(automatic=True, compute='_compute_display_name', search=...)` | |
| with no store. An override that omits a dependency therefore cannot leave | |
| a stale row, because there is no row. | |
| The checker read "no declaration in this file" as unknown storedness and | |
| kept the finding, which is right for an ordinary field declared in another | |
| module and wrong here: the framework's declaration is knowable and says | |
| not stored. 121 of the 1,051 findings this rule made on odoo core were | |
| this shape. | |
| """ | |
| body = ("class M(models.Model):\n" | |
| " _inherit = 'res.partner'\n" | |
| "\n" | |
| " @api.depends('phone')\n" | |
| " def _compute_display_name(self):\n" | |
| " for rec in self:\n" | |
| " rec.display_name = rec.name + rec.phone\n") | |
| assert semantics.run_check("depends_covers_compute_reads", body) == [] | |
| def test_a_model_that_stores_display_name_itself_is_still_checked(): | |
| """The narrowing is about the framework's default, not about the name. A | |
| model that declares `display_name = fields.Char(..., store=True)` in its | |
| own file has opted into a stored row, and all 10 hand-verified true | |
| positives that compute display_name -- every one of them in OpenSPP -- do | |
| exactly that. Losing those is what this test exists to prevent. | |
| """ | |
| body = ("class M(models.Model):\n" | |
| " _name = 'a.b'\n" | |
| "\n" | |
| " display_name = fields.Char(compute='_compute_display_name', store=True)\n" | |
| "\n" | |
| " @api.depends('phone')\n" | |
| " def _compute_display_name(self):\n" | |
| " for rec in self:\n" | |
| " rec.display_name = rec.name + rec.phone\n") | |
| found = semantics.run_check("depends_covers_compute_reads", body) | |
| assert any("'name'" in message for _, message, _ in found), found | |
| def test_a_sql_view_model_has_no_foreign_key_to_decide_about(): | |
| """`_auto = False` means odoo builds no table for the model, so there is | |
| no foreign key, and `ondelete` is not a decision anybody can make about | |
| it. Two of the 15 sampled ondelete findings on odoo core were report | |
| models of exactly this kind: hr.contract.history and | |
| purchase.bill.line.match. | |
| None of the 26 hand-verified true positives for this rule sits on an | |
| _auto = False model, which is what makes the exemption free. | |
| """ | |
| view = ("class R(models.Model):\n" | |
| " _name = 'a.report'\n" | |
| " _auto = False\n" | |
| "\n" | |
| " employee_id = fields.Many2one('hr.employee', readonly=True)\n" | |
| "\n" | |
| " def f(self):\n" | |
| " return self.employee_id.name\n") | |
| assert semantics.run_check("ondelete_leaves_a_dangling_read", view) == [] | |
| table = view.replace(" _auto = False\n", "") | |
| assert semantics.run_check("ondelete_leaves_a_dangling_read", table), \ | |
| "a real table with the same declaration is still the rule's subject" | |
| def test_sudo_de_escalation_is_recognised_by_keyword_too(): | |
| """odoo's signature is `sudo(self, flag=True)`, so `sudo(flag=False)` is | |
| the same de-escalation as `sudo(False)`. Reading only positional | |
| arguments missed it, and core writes it that way twice.""" | |
| body = "def f(self):\n return self.env['res.partner'].sudo(flag=False).search([])\n" | |
| assert semantics.run_check("sudo_drops_record_rules", body) == [] | |
| def test_restoring_the_ambient_level_is_not_an_escalation(): | |
| """`x.sudo(self.env.su)` sets su to whatever the environment already has, | |
| so it cannot raise privilege above ambient. It is how a method hands a | |
| record back at the caller's own level, and odoo core writes it beside the | |
| comment "Unsudo the invoice after creation if not already sudoed". | |
| `sudo(self.env.su or field.compute_sudo)` is not the same thing: the `or` | |
| can turn it on, so only the bare attribute reads as ambient. | |
| """ | |
| ambient = "def f(self, invoice):\n invoice = invoice.sudo(self.env.su)\n return invoice\n" | |
| assert semantics.run_check("sudo_drops_record_rules", ambient) == [] | |
| may_raise = ("def f(self, rec):\n" | |
| " rec = rec.sudo(self.env.su or self.compute_sudo)\n" | |
| " return rec\n") | |
| assert semantics.run_check("sudo_drops_record_rules", may_raise) | |
| # ------------------------------------------------------ parent_can_form_a_cycle | |
| def _hierarchy(header, field='parent_id = fields.Many2one("my.category", "Parent")'): | |
| return ("from odoo import fields, models\n\n\n" | |
| "class Category(models.Model):\n" | |
| ' _name = "my.category"\n' + header + | |
| " " + field + "\n") | |
| def test_a_self_referencing_parent_with_no_guard_is_caught(): | |
| """The rule's subject: spp.dms.directory, which declares a parent_path | |
| Char but never turns _parent_store on, so nothing refuses a loop.""" | |
| found = semantics.run_check("parent_can_form_a_cycle", _hierarchy("")) | |
| assert [line for line, _, _ in found] == [6] | |
| assert "my.category" in found[0][1] | |
| def test_parent_store_already_refuses_the_cycle(): | |
| """odoo's _parent_store_update raises "Recursion Detected." on a write that | |
| would close a loop. Six of seven verdicts once classified true were this.""" | |
| body = _hierarchy(" _parent_store = True\n") | |
| assert semantics.run_check("parent_can_form_a_cycle", body) == [] | |
| def test_parent_store_on_another_chain_leaves_parent_id_bare(): | |
| body = _hierarchy(' _parent_store = True\n _parent_name = "location_id"\n') | |
| assert semantics.run_check("parent_can_form_a_cycle", body) | |
| def test_a_parent_on_another_model_cannot_close_a_loop(): | |
| """OCA's account.cash.deposit.line.parent_id points at account.cash.deposit: | |
| an ordinary foreign key, classified false as wrong-construct.""" | |
| body = _hierarchy("", 'parent_id = fields.Many2one("my.batch", ondelete="cascade")') | |
| assert semantics.run_check("parent_can_form_a_cycle", body) == [] | |
| def test_self_reference_is_read_through_name_and_a_module_constant(): | |
| """spp.area writes Many2one(_name, ...); spp.area.type assigns both _name | |
| and the comodel from one module constant.""" | |
| by_name = _hierarchy("", 'parent_id = fields.Many2one(_name, "Parent")') | |
| assert semantics.run_check("parent_can_form_a_cycle", by_name) | |
| shared = ('from odoo import fields, models\n\n_type_model = "my.type"\n\n\n' | |
| "class AreaType(models.Model):\n" | |
| " _name = _type_model\n" | |
| ' parent_id = fields.Many2one(_type_model, "Parent")\n') | |
| assert semantics.run_check("parent_can_form_a_cycle", shared) | |
| elsewhere = shared.replace('fields.Many2one(_type_model', 'fields.Many2one("my.zone"') | |
| assert semantics.run_check("parent_can_form_a_cycle", elsewhere) == [] | |
| def test_either_guard_clears_the_hierarchy_rule(): | |
| for guard in ("_has_cycle", "_check_recursion"): | |
| body = _hierarchy("") + ( | |
| "\n @api.constrains('parent_id')\n" | |
| " def _check_parent(self):\n" | |
| " if self.{0}():\n" | |
| " raise ValidationError('loop')\n").format(guard) | |
| assert semantics.run_check("parent_can_form_a_cycle", body) == [], guard | |
| def test_a_chained_name_assignment_is_still_the_models_name(): | |
| """odoo's test models write `_name = _description = 'x'`. Read as nameless, | |
| a parent_id to a different model fired as if it could loop.""" | |
| body = ("from odoo import fields, models\n\n\n" | |
| "class Child(models.Model):\n" | |
| " _name = _description = 'my.child'\n" | |
| " parent_id = fields.Many2one('my.parent')\n") | |
| assert semantics.run_check("parent_can_form_a_cycle", body) == [] | |
| def test_a_view_and_a_redeclaration_are_not_the_hierarchy_rules_subject(): | |
| view = _hierarchy(" _auto = False\n") | |
| assert semantics.run_check("parent_can_form_a_cycle", view) == [] | |
| override = _hierarchy("", "parent_id = fields.Many2one(tracking=3)") | |
| assert semantics.run_check("parent_can_form_a_cycle", override) == [] | |
| # ------------------------------------------------ editable_record_without_noupdate | |
| PARAMS = """<?xml version="1.0" encoding="utf-8" ?> | |
| <odoo> | |
| <record id="view_thing_form" model="ir.ui.view"> | |
| <field name="name">thing.form</field> | |
| </record> | |
| <record id="retention_days" model="ir.config_parameter"> | |
| <field name="key">my.retention_days</field> | |
| <field name="value">90</field> | |
| </record> | |
| <data noupdate="1"> | |
| <record id="seq_thing" model="ir.sequence"> | |
| <field name="name">Thing</field> | |
| </record> | |
| </data> | |
| </odoo> | |
| """ | |
| def test_an_editable_record_outside_noupdate_is_reported_on_its_own_line(): | |
| """The regex this replaced fired once, on <odoo>, for any record at all -- | |
| and one noupdate block anywhere cleared the whole file.""" | |
| found = semantics.run_check("editable_record_without_noupdate", PARAMS) | |
| assert [line for line, _, _ in found] == [6] | |
| assert "retention_days" in found[0][1] and "migration" in found[0][1] | |
| def test_a_view_is_not_the_noupdate_rules_subject(): | |
| """Views, menus and actions have to upgrade: core keeps 0-2% of them under | |
| noupdate, so reporting them would be advice nobody should take.""" | |
| body = PARAMS.replace('model="ir.config_parameter"', 'model="ir.actions.act_window"') | |
| assert semantics.run_check("editable_record_without_noupdate", body) == [] | |
| def test_noupdate_is_inherited_and_can_be_switched_back_off(): | |
| root = PARAMS.replace("<odoo>", '<odoo noupdate="1">') | |
| assert semantics.run_check("editable_record_without_noupdate", root) == [] | |
| inner = root.replace('<data noupdate="1">', '<data noupdate="0">') | |
| assert [l for l, _, _ in semantics.run_check( | |
| "editable_record_without_noupdate", inner)] == [11] | |
| def test_stages_and_precisions_are_editable_data_too(): | |
| for model in ("crm.stage", "decimal.precision", "ir.sequence"): | |
| body = PARAMS.replace('model="ir.config_parameter"', 'model="{0}"'.format(model)) | |
| assert semantics.run_check("editable_record_without_noupdate", body), model | |
| def test_a_cron_counts_only_when_it_ships_disabled(): | |
| """57 of 62 cron findings read false: a schedule is usually the mechanism. | |
| The shape that held up ships active=False for an operator to switch on, | |
| which is the decision an upgrade then undoes.""" | |
| running = PARAMS.replace('model="ir.config_parameter"', 'model="ir.cron"') | |
| assert semantics.run_check("editable_record_without_noupdate", running) == [] | |
| for spelling in ('<field name="active" eval="False"/>', | |
| '<field name="active">0</field>'): | |
| disabled = running.replace('<field name="value">90</field>', spelling) | |
| assert semantics.run_check("editable_record_without_noupdate", disabled), spelling | |
| def test_xml_inside_an_answer_is_still_read(): | |
| """A generated answer carries its XML among prose and Python; the whole text | |
| does not parse, the document inside it does.""" | |
| answer = "Add this data file:\n\n" + PARAMS.split("\n", 1)[1] + "\nand then def f(): pass\n" | |
| found = semantics.run_check("editable_record_without_noupdate", answer) | |
| # Line 7 of the answer: two lines of prose, and the declaration dropped. | |
| assert [line for line, _, _ in found] == [7] | |
| def test_check_company_is_not_asked_of_a_sql_view(): | |
| """A SQL view has no table and so no foreign key to guard; OpenSPP's | |
| fund_report was this rule's last live false positive.""" | |
| body = ("from odoo import fields, models\n\n\n" | |
| "class FundReport(models.Model):\n" | |
| " _name = 'my.fund.report'\n" | |
| " _auto = False\n" | |
| " company_id = fields.Many2one('res.company')\n" | |
| " journal_id = fields.Many2one('account.journal')\n") | |
| assert semantics.run_check("check_company_missing_on_company_owned_relation", body) == [] | |
| table = body.replace(" _auto = False\n", "") | |
| assert semantics.run_check("check_company_missing_on_company_owned_relation", table) | |
| def test_a_field_read_only_to_log_is_not_a_dependency(): | |
| """Five of OpenSPP's live false positives read rec.name only to phrase a | |
| log line. A value read to phrase something decides nothing.""" | |
| body = ("from odoo import api, models\n\n\n" | |
| "class Thing(models.Model):\n" | |
| " _name = 'my.thing'\n\n" | |
| " @api.constrains('source_type')\n" | |
| " def _check_source(self):\n" | |
| " for rec in self:\n" | |
| " if rec.source_type == 'external':\n" | |
| " _logger.warning('thing %s has no provider', rec.name)\n") | |
| assert semantics.run_check("constrains_lists_every_field_read", body) == [] | |
| decides = body.replace("if rec.source_type == 'external':", | |
| "if rec.source_type == 'external' and rec.name:") | |
| found = semantics.run_check("constrains_lists_every_field_read", decides) | |
| assert found and "'name'" in found[0][1], "a read in the condition still counts" | |
| def test_a_message_collected_before_it_is_raised_is_still_a_message(): | |
| """odoo core's event seats check appends each line to a list and raises | |
| the joined list; event.name only ever phrases it.""" | |
| body = ("from odoo import api, models\n\n\n" | |
| "class Event(models.Model):\n" | |
| " _name = 'my.event'\n\n" | |
| " @api.constrains('seats_max')\n" | |
| " def _check_seats(self):\n" | |
| " sold_out = []\n" | |
| " for event in self:\n" | |
| " if event.seats_max < 0:\n" | |
| " sold_out.append(_('%(n)s', n=event.name))\n" | |
| " if sold_out:\n" | |
| " raise ValidationError('\n'.join(sold_out))\n") | |
| assert semantics.run_check("constrains_lists_every_field_read", body) == [] | |
| def test_an_override_that_calls_super_adds_triggers_for_the_chain(): | |
| """Odoo merges @api.depends across overrides, so an override that calls | |
| super() is adding triggers for fields the parent's body reads. Three of | |
| core's sampled depends-unread findings were this, the partner avatars | |
| among them.""" | |
| body = ("from odoo import api, models\n\n\n" | |
| "class Partner(models.Model):\n" | |
| " _inherit = 'res.partner'\n\n" | |
| " @api.depends('name', 'image_128', 'is_company')\n" | |
| " def _compute_avatar_128(self):\n" | |
| " super()._compute_avatar_128()\n") | |
| assert semantics.run_check("depends_names_unread_fields", body) == [] | |
| def test_a_body_that_only_assigns_a_constant_stays_a_finding(): | |
| """Tried as an exemption and withdrawn: on core such bodies were stubs or | |
| deliberate resets, on OpenSPP and OCA three were leftovers classified true | |
| by hand. The file cannot tell which.""" | |
| body = ("from odoo import api, fields, models\n\n\n" | |
| "class Field(models.Model):\n" | |
| " _name = 'my.field'\n\n" | |
| " @api.depends('target_type')\n" | |
| " def _compute_target_model(self):\n" | |
| " for record in self:\n" | |
| " record.target_model = 'res.partner'\n") | |
| assert semantics.run_check("depends_names_unread_fields", body) | |
| _API = "from datetime import datetime\nfrom fastapi import APIRouter\n\n\n" | |
| def test_datetime_reports_the_two_shapes_that_survived(why, body): | |
| assert semantics.run_check("naive_time_leaves_or_dates_a_day", body), why | |
| def test_datetime_leaves_the_shapes_that_did_not(why, body): | |
| assert semantics.run_check("naive_time_leaves_or_dates_a_day", body) == [], why | |
| _LEGACY = ("from odoo import models\n\n\n" | |
| "class ReasonDocument(models.Model):\n" | |
| " _name = 'spp.cr.type.reason.document'\n\n" | |
| " _sql_constraints = [\n" | |
| " ('reason_uniq', 'UNIQUE(cr_type_id, reason)', 'One rule per reason.'),\n" | |
| " ]\n") | |
| def test_sql_constraints_on_19_are_a_constraint_that_does_not_exist(): | |
| """OpenSPP's 208d9758: the block sat on a 19.0 module and created nothing.""" | |
| found = semantics.run_check("legacy_sql_constraints_on_19", _LEGACY, | |
| facts={"walked": True, "odoo_series": "19.0"}) | |
| assert [(line, grade) for line, _, grade in found] == [(7, None)] | |
| def test_sql_constraints_before_19_are_the_only_spelling(): | |
| """18.0 core declares the attribute in 174 files and has no | |
| models.Constraint, so a finding there is advice that cannot be taken.""" | |
| for series in ("16.0", "17.0", "18.0"): | |
| assert semantics.run_check("legacy_sql_constraints_on_19", _LEGACY, | |
| facts={"walked": True, "odoo_series": series}) == [] | |
| def test_sql_constraints_series_unknown_after_a_walk_is_silent(): | |
| """odoo core's own manifests pin no series; 174 findings there would all | |
| be wrong.""" | |
| assert semantics.run_check("legacy_sql_constraints_on_19", _LEGACY, | |
| facts={"walked": True, "odoo_series": None}) == [] | |
| def test_sql_constraints_with_nothing_walked_is_advisory(): | |
| """Nobody looked is not the same answer as looked and it does not say.""" | |
| found = semantics.run_check("legacy_sql_constraints_on_19", _LEGACY) | |
| assert [grade for _, _, grade in found] == [semantics.UNPROVEN] | |
| def test_an_empty_sql_constraints_list_declares_nothing(): | |
| body = ("from odoo import models\n\n\n" | |
| "class Thing(models.Model):\n" | |
| " _name = 'my.thing'\n" | |
| " _sql_constraints = []\n") | |
| assert semantics.run_check("legacy_sql_constraints_on_19", body, | |
| facts={"walked": True, "odoo_series": "19.0"}) == [] | |
| def _sudo_in_method(body_line, params="self"): | |
| return ("from odoo import models\n\n\n" | |
| "class Employee(models.Model):\n" | |
| " _inherit = 'hr.employee'\n\n" | |
| " def f(" + params + "):\n" | |
| " " + body_line + "\n") | |
| def test_sudo_tied_to_held_records_is_re_scoped(why, line): | |
| assert semantics.run_check("sudo_drops_record_rules", _sudo_in_method(line)) == [], why | |
| def test_sudo_not_tied_to_held_records_stays_a_finding(why, line, params): | |
| assert semantics.run_check("sudo_drops_record_rules", _sudo_in_method(line, params)), why | |
| def _sudo_on_a_page(body, auth="public", website="True"): | |
| return ("from odoo import http\n" | |
| "from odoo.http import request\n\n\n" | |
| "class Customers(http.Controller):\n\n" | |
| " @http.route(['/customers'], type='http', auth=" + repr(auth) | |
| + ", website=" + website + ")\n" | |
| " def customers(self, search=None, **post):\n" | |
| + "".join(" " + line + "\n" for line in body)) | |
| def test_sudo_published_on_a_public_page_is_re_scoped(why, body): | |
| assert semantics.run_check("sudo_drops_record_rules", _sudo_on_a_page(body)) == [], why | |
| def test_sudo_published_not_on_a_public_page_stays_a_finding(why, body, auth, website): | |
| found = semantics.run_check("sudo_drops_record_rules", | |
| _sudo_on_a_page(body, auth, website)) | |
| assert found, why | |
| # --------------------------------- a constraint reading a field it cannot name | |
| def _seats(declaration, depends, constrains, compute_name="_compute_seats"): | |
| return ("from odoo import api, fields, models\n\n\n" | |
| "class Event(models.Model):\n" | |
| " _name = 'my.event'\n\n" | |
| " seats_max = fields.Integer()\n" | |
| " seats_taken = fields.Integer()\n" | |
| " seats_available = fields.Integer({0})\n\n" | |
| " @api.depends({1})\n" | |
| " def {3}(self):\n" | |
| " for event in self:\n" | |
| " event.seats_available = event.seats_max - event.seats_taken\n\n" | |
| " @api.constrains({2})\n" | |
| " def _check_seats(self):\n" | |
| " for event in self:\n" | |
| " if event.seats_available < 0:\n" | |
| " raise ValidationError('sold out')\n").format( | |
| declaration, depends, constrains, compute_name) | |
| def test_a_constraint_naming_every_input_of_an_unstored_field_is_covered(): | |
| """Odoo never runs a constraint for a field that is neither stored nor | |
| inversible, and logs "@constrains parameter ... is not writeable" when one | |
| is named. A check reading seats_available that names what it is computed | |
| from runs whenever the value can change, so asking for seats_available was | |
| asking for a name that does nothing. core's event seats are two of the | |
| eight findings that had this shape (ARCHITECTURE 3.82).""" | |
| body = _seats("compute='_compute_seats'", "'seats_max', 'seats_taken'", | |
| "'seats_max', 'seats_taken'") | |
| assert semantics.run_check("constrains_lists_every_field_read", body) == [] | |
| def test_a_constraint_missing_an_input_is_told_which_one(): | |
| body = _seats("compute='_compute_seats'", "'seats_max', 'seats_taken'", | |
| "'seats_max'") | |
| found = semantics.run_check("constrains_lists_every_field_read", body) | |
| assert len(found) == 1 | |
| message = found[0][1] | |
| assert "not stored" in message and "writing 'seats_taken' alone" in message | |
| assert "writing 'seats_available'" not in message, "advice that cannot be taken" | |
| def test_a_stored_or_inversible_field_is_still_asked_for(declaration): | |
| """Stored, a recompute validates it; inversible, a write of it runs the | |
| check. Naming it works either way, so the finding asks for it as before.""" | |
| body = _seats(declaration, "'seats_max', 'seats_taken'", | |
| "'seats_max', 'seats_taken'") | |
| found = semantics.run_check("constrains_lists_every_field_read", body) | |
| assert len(found) == 1 and "writing 'seats_available' alone" in found[0][1] | |
| def test_a_related_field_is_reached_through_its_first_field(extra, quiet): | |
| """A related field is readonly unless it says otherwise, and only then | |
| gets an inverse. Readonly, the relation it starts from is what a write | |
| can touch.""" | |
| body = ("from odoo import api, fields, models\n\n\n" | |
| "class Line(models.Model):\n" | |
| " _name = 'my.line'\n\n" | |
| " partner_id = fields.Many2one('res.partner')\n" | |
| " country_code = fields.Char(related='partner_id.country_id.code'" | |
| + extra + ")\n\n" | |
| " @api.constrains('partner_id')\n" | |
| " def _check_country(self):\n" | |
| " for line in self:\n" | |
| " if line.country_code == 'XX':\n" | |
| " raise ValidationError('no')\n") | |
| found = semantics.run_check("constrains_lists_every_field_read", body) | |
| assert (found == []) is quiet, found | |
| def test_an_unstored_input_is_followed_to_what_it_is_computed_from(): | |
| body = _seats("compute='_compute_seats'", "'seats_max', 'seats_reserved'", | |
| "'seats_max'").replace( | |
| " seats_taken = fields.Integer()\n", | |
| " seats_taken = fields.Integer()\n" | |
| " seats_reserved = fields.Integer(compute='_compute_reserved')\n\n" | |
| " @api.depends('registration_ids')\n" | |
| " def _compute_reserved(self):\n" | |
| " pass\n\n") | |
| found = semantics.run_check("constrains_lists_every_field_read", body) | |
| assert len(found) == 1 | |
| assert "writing 'registration_ids' alone" in found[0][1] | |
| assert "'seats_reserved'" not in found[0][1].split("--")[1] | |
| def test_an_unstored_field_whose_inputs_cannot_be_read_says_so(): | |
| """The compute lives elsewhere, so what it reads is unknown. The finding | |
| still stands, and says the field itself cannot be named.""" | |
| body = _seats("compute='_compute_elsewhere'", "'seats_max', 'seats_taken'", | |
| "'seats_max'") | |
| found = semantics.run_check("constrains_lists_every_field_read", body) | |
| assert len(found) == 1 | |
| assert "not stored" in found[0][1] | |
| assert "writing 'seats_available'" not in found[0][1] | |