File size: 237,327 Bytes
d3cfa77 666f897 d3cfa77 3e7a0a4 d3cfa77 666f897 d3cfa77 6faed22 d3cfa77 03128a2 177f1ca d3cfa77 03128a2 177f1ca 03128a2 d3cfa77 03128a2 d3cfa77 0539253 a8f9476 d3cfa77 03128a2 d3cfa77 03128a2 d3cfa77 223d4f4 a9e151c d3cfa77 223d4f4 d3cfa77 a9e151c 223d4f4 a9e151c d3cfa77 a9e151c d3cfa77 c3d4547 d3cfa77 03128a2 66c7faa 03128a2 054a97a a8f9476 66c7faa 03128a2 66c7faa 03128a2 66c7faa 03128a2 66c7faa 03128a2 223d4f4 7dbbe3a 53c8de6 223d4f4 7dbbe3a 223d4f4 d3cfa77 223d4f4 7dbbe3a d3cfa77 223d4f4 d3cfa77 7dbbe3a 03128a2 82013bf d3cfa77 223d4f4 d3cfa77 03128a2 d3cfa77 223d4f4 82013bf d3cfa77 82013bf d3cfa77 82013bf d3cfa77 82013bf 5fdf138 82013bf 5fdf138 82013bf 7dbbe3a 223d4f4 210b09e 5fdf138 d3cfa77 ab8d335 223d4f4 ab8d335 0bc85c5 ab8d335 0bc85c5 ab8d335 c3d4547 0bc85c5 b999a8e 0bc85c5 054a97a c3d4547 66c7faa 03128a2 66c7faa 03128a2 5fdf138 d3cfa77 c3d4547 054a97a 5fdf138 d3cfa77 223d4f4 c3d4547 223d4f4 5fdf138 ab8d335 210b09e d3cfa77 66c7faa 054a97a 03128a2 ab8d335 054a97a ab8d335 d3cfa77 5fdf138 d3cfa77 f6ab2f2 d3cfa77 5fdf138 ab8d335 c3d4547 054a97a ab8d335 6d991e1 223d4f4 6d991e1 e9d9bcb 223d4f4 6d991e1 223d4f4 6d991e1 223d4f4 6d991e1 223d4f4 e9d9bcb 223d4f4 5ac1be2 624fbef 5ac1be2 6d991e1 5ac1be2 223d4f4 5ac1be2 223d4f4 5ac1be2 6d991e1 5ac1be2 6d991e1 5ac1be2 223d4f4 5ac1be2 6d991e1 5ac1be2 ab8d335 5ac1be2 223d4f4 5ac1be2 6faed22 7d84af7 223d4f4 7d84af7 6faed22 7d84af7 6faed22 7d84af7 6faed22 7d84af7 223d4f4 7d84af7 223d4f4 7d84af7 6faed22 7d84af7 210b09e 223d4f4 210b09e 223d4f4 210b09e 223d4f4 210b09e 223d4f4 7d84af7 6d991e1 7d84af7 6d991e1 d0bca75 7d84af7 6d991e1 7d84af7 6d991e1 7d84af7 6d991e1 c3d4547 6faed22 c3d4547 6faed22 c3d4547 a9e151c c3d4547 223d4f4 6faed22 c3d4547 c298898 666f897 405d61b 223d4f4 405d61b 223d4f4 405d61b 223d4f4 405d61b 3e7a0a4 405d61b 03128a2 405d61b 223d4f4 405d61b 575ff59 405d61b 03128a2 405d61b 575ff59 405d61b 6faed22 f02c614 6faed22 5fdf138 d904a25 5fdf138 6faed22 f02c614 6faed22 f02c614 6faed22 210b09e 9012cfa 210b09e 9012cfa 210b09e 9012cfa 210b09e 9012cfa 210b09e 9012cfa 210b09e 9012cfa 210b09e 9012cfa 210b09e 9012cfa 210b09e 9012cfa ae568da 210b09e 9012cfa 210b09e 9012cfa 77aa4e0 fed0255 6eb635c 210b09e 77aa4e0 210b09e 77aa4e0 6eb635c 77aa4e0 fed0255 210b09e 77aa4e0 210b09e 77aa4e0 210b09e fed0255 210b09e 77aa4e0 210b09e a9e151c d43fe3b 847fcae 210b09e d43fe3b 847fcae 210b09e a9e151c 210b09e 847fcae d43fe3b 0e2212c 210b09e 0e2212c 210b09e a9e151c 210b09e a9e151c 210b09e 9012cfa d3cfa77 3e7a0a4 210b09e c3d4547 405d61b d3cfa77 7dbbe3a d3cfa77 7d84af7 5ac1be2 c298898 666f897 ae568da e9d9bcb d3cfa77 6faed22 9012cfa 210b09e 223d4f4 e9d9bcb 6eb635c 5fdf138 6faed22 223d4f4 7dbbe3a 223d4f4 6faed22 223d4f4 d3cfa77 6faed22 223d4f4 6faed22 d3cfa77 5fdf138 6faed22 223d4f4 6faed22 d3cfa77 cbaf04a d3cfa77 5ac1be2 d3cfa77 210b09e d3cfa77 210b09e d3cfa77 210b09e d3cfa77 210b09e d3cfa77 210b09e d3cfa77 210b09e d3cfa77 223d4f4 d3cfa77 223d4f4 d3cfa77 223d4f4 d3cfa77 223d4f4 d3cfa77 223d4f4 d3cfa77 | 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451 452 453 454 455 456 457 458 459 460 461 462 463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489 490 491 492 493 494 495 496 497 498 499 500 501 502 503 504 505 506 507 508 509 510 511 512 513 514 515 516 517 518 519 520 521 522 523 524 525 526 527 528 529 530 531 532 533 534 535 536 537 538 539 540 541 542 543 544 545 546 547 548 549 550 551 552 553 554 555 556 557 558 559 560 561 562 563 564 565 566 567 568 569 570 571 572 573 574 575 576 577 578 579 580 581 582 583 584 585 586 587 588 589 590 591 592 593 594 595 596 597 598 599 600 601 602 603 604 605 606 607 608 609 610 611 612 613 614 615 616 617 618 619 620 621 622 623 624 625 626 627 628 629 630 631 632 633 634 635 636 637 638 639 640 641 642 643 644 645 646 647 648 649 650 651 652 653 654 655 656 657 658 659 660 661 662 663 664 665 666 667 668 669 670 671 672 673 674 675 676 677 678 679 680 681 682 683 684 685 686 687 688 689 690 691 692 693 694 695 696 697 698 699 700 701 702 703 704 705 706 707 708 709 710 711 712 713 714 715 716 717 718 719 720 721 722 723 724 725 726 727 728 729 730 731 732 733 734 735 736 737 738 739 740 741 742 743 744 745 746 747 748 749 750 751 752 753 754 755 756 757 758 759 760 761 762 763 764 765 766 767 768 769 770 771 772 773 774 775 776 777 778 779 780 781 782 783 784 785 786 787 788 789 790 791 792 793 794 795 796 797 798 799 800 801 802 803 804 805 806 807 808 809 810 811 812 813 814 815 816 817 818 819 820 821 822 823 824 825 826 827 828 829 830 831 832 833 834 835 836 837 838 839 840 841 842 843 844 845 846 847 848 849 850 851 852 853 854 855 856 857 858 859 860 861 862 863 864 865 866 867 868 869 870 871 872 873 874 875 876 877 878 879 880 881 882 883 884 885 886 887 888 889 890 891 892 893 894 895 896 897 898 899 900 901 902 903 904 905 906 907 908 909 910 911 912 913 914 915 916 917 918 919 920 921 922 923 924 925 926 927 928 929 930 931 932 933 934 935 936 937 938 939 940 941 942 943 944 945 946 947 948 949 950 951 952 953 954 955 956 957 958 959 960 961 962 963 964 965 966 967 968 969 970 971 972 973 974 975 976 977 978 979 980 981 982 983 984 985 986 987 988 989 990 991 992 993 994 995 996 997 998 999 1000 1001 1002 1003 1004 1005 1006 1007 1008 1009 1010 1011 1012 1013 1014 1015 1016 1017 1018 1019 1020 1021 1022 1023 1024 1025 1026 1027 1028 1029 1030 1031 1032 1033 1034 1035 1036 1037 1038 1039 1040 1041 1042 1043 1044 1045 1046 1047 1048 1049 1050 1051 1052 1053 1054 1055 1056 1057 1058 1059 1060 1061 1062 1063 1064 1065 1066 1067 1068 1069 1070 1071 1072 1073 1074 1075 1076 1077 1078 1079 1080 1081 1082 1083 1084 1085 1086 1087 1088 1089 1090 1091 1092 1093 1094 1095 1096 1097 1098 1099 1100 1101 1102 1103 1104 1105 1106 1107 1108 1109 1110 1111 1112 1113 1114 1115 1116 1117 1118 1119 1120 1121 1122 1123 1124 1125 1126 1127 1128 1129 1130 1131 1132 1133 1134 1135 1136 1137 1138 1139 1140 1141 1142 1143 1144 1145 1146 1147 1148 1149 1150 1151 1152 1153 1154 1155 1156 1157 1158 1159 1160 1161 1162 1163 1164 1165 1166 1167 1168 1169 1170 1171 1172 1173 1174 1175 1176 1177 1178 1179 1180 1181 1182 1183 1184 1185 1186 1187 1188 1189 1190 1191 1192 1193 1194 1195 1196 1197 1198 1199 1200 1201 1202 1203 1204 1205 1206 1207 1208 1209 1210 1211 1212 1213 1214 1215 1216 1217 1218 1219 1220 1221 1222 1223 1224 1225 1226 1227 1228 1229 1230 1231 1232 1233 1234 1235 1236 1237 1238 1239 1240 1241 1242 1243 1244 1245 1246 1247 1248 1249 1250 1251 1252 1253 1254 1255 1256 1257 1258 1259 1260 1261 1262 1263 1264 1265 1266 1267 1268 1269 1270 1271 1272 1273 1274 1275 1276 1277 1278 1279 1280 1281 1282 1283 1284 1285 1286 1287 1288 1289 1290 1291 1292 1293 1294 1295 1296 1297 1298 1299 1300 1301 1302 1303 1304 1305 1306 1307 1308 1309 1310 1311 1312 1313 1314 1315 1316 1317 1318 1319 1320 1321 1322 1323 1324 1325 1326 1327 1328 1329 1330 1331 1332 1333 1334 1335 1336 1337 1338 1339 1340 1341 1342 1343 1344 1345 1346 1347 1348 1349 1350 1351 1352 1353 1354 1355 1356 1357 1358 1359 1360 1361 1362 1363 1364 1365 1366 1367 1368 1369 1370 1371 1372 1373 1374 1375 1376 1377 1378 1379 1380 1381 1382 1383 1384 1385 1386 1387 1388 1389 1390 1391 1392 1393 1394 1395 1396 1397 1398 1399 1400 1401 1402 1403 1404 1405 1406 1407 1408 1409 1410 1411 1412 1413 1414 1415 1416 1417 1418 1419 1420 1421 1422 1423 1424 1425 1426 1427 1428 1429 1430 1431 1432 1433 1434 1435 1436 1437 1438 1439 1440 1441 1442 1443 1444 1445 1446 1447 1448 1449 1450 1451 1452 1453 1454 1455 1456 1457 1458 1459 1460 1461 1462 1463 1464 1465 1466 1467 1468 1469 1470 1471 1472 1473 1474 1475 1476 1477 1478 1479 1480 1481 1482 1483 1484 1485 1486 1487 1488 1489 1490 1491 1492 1493 1494 1495 1496 1497 1498 1499 1500 1501 1502 1503 1504 1505 1506 1507 1508 1509 1510 1511 1512 1513 1514 1515 1516 1517 1518 1519 1520 1521 1522 1523 1524 1525 1526 1527 1528 1529 1530 1531 1532 1533 1534 1535 1536 1537 1538 1539 1540 1541 1542 1543 1544 1545 1546 1547 1548 1549 1550 1551 1552 1553 1554 1555 1556 1557 1558 1559 1560 1561 1562 1563 1564 1565 1566 1567 1568 1569 1570 1571 1572 1573 1574 1575 1576 1577 1578 1579 1580 1581 1582 1583 1584 1585 1586 1587 1588 1589 1590 1591 1592 1593 1594 1595 1596 1597 1598 1599 1600 1601 1602 1603 1604 1605 1606 1607 1608 1609 1610 1611 1612 1613 1614 1615 1616 1617 1618 1619 1620 1621 1622 1623 1624 1625 1626 1627 1628 1629 1630 1631 1632 1633 1634 1635 1636 1637 1638 1639 1640 1641 1642 1643 1644 1645 1646 1647 1648 1649 1650 1651 1652 1653 1654 1655 1656 1657 1658 1659 1660 1661 1662 1663 1664 1665 1666 1667 1668 1669 1670 1671 1672 1673 1674 1675 1676 1677 1678 1679 1680 1681 1682 1683 1684 1685 1686 1687 1688 1689 1690 1691 1692 1693 1694 1695 1696 1697 1698 1699 1700 1701 1702 1703 1704 1705 1706 1707 1708 1709 1710 1711 1712 1713 1714 1715 1716 1717 1718 1719 1720 1721 1722 1723 1724 1725 1726 1727 1728 1729 1730 1731 1732 1733 1734 1735 1736 1737 1738 1739 1740 1741 1742 1743 1744 1745 1746 1747 1748 1749 1750 1751 1752 1753 1754 1755 1756 1757 1758 1759 1760 1761 1762 1763 1764 1765 1766 1767 1768 1769 1770 1771 1772 1773 1774 1775 1776 1777 1778 1779 1780 1781 1782 1783 1784 1785 1786 1787 1788 1789 1790 1791 1792 1793 1794 1795 1796 1797 1798 1799 1800 1801 1802 1803 1804 1805 1806 1807 1808 1809 1810 1811 1812 1813 1814 1815 1816 1817 1818 1819 1820 1821 1822 1823 1824 1825 1826 1827 1828 1829 1830 1831 1832 1833 1834 1835 1836 1837 1838 1839 1840 1841 1842 1843 1844 1845 1846 1847 1848 1849 1850 1851 1852 1853 1854 1855 1856 1857 1858 1859 1860 1861 1862 1863 1864 1865 1866 1867 1868 1869 1870 1871 1872 1873 1874 1875 1876 1877 1878 1879 1880 1881 1882 1883 1884 1885 1886 1887 1888 1889 1890 1891 1892 1893 1894 1895 1896 1897 1898 1899 1900 1901 1902 1903 1904 1905 1906 1907 1908 1909 1910 1911 1912 1913 1914 1915 1916 1917 1918 1919 1920 1921 1922 1923 1924 1925 1926 1927 1928 1929 1930 1931 1932 1933 1934 1935 1936 1937 1938 1939 1940 1941 1942 1943 1944 1945 1946 1947 1948 1949 1950 1951 1952 1953 1954 1955 1956 1957 1958 1959 1960 1961 1962 1963 1964 1965 1966 1967 1968 1969 1970 1971 1972 1973 1974 1975 1976 1977 1978 1979 1980 1981 1982 1983 1984 1985 1986 1987 1988 1989 1990 1991 1992 1993 1994 1995 1996 1997 1998 1999 2000 2001 2002 2003 2004 2005 2006 2007 2008 2009 2010 2011 2012 2013 2014 2015 2016 2017 2018 2019 2020 2021 2022 2023 2024 2025 2026 2027 2028 2029 2030 2031 2032 2033 2034 2035 2036 2037 2038 2039 2040 2041 2042 2043 2044 2045 2046 2047 2048 2049 2050 2051 2052 2053 2054 2055 2056 2057 2058 2059 2060 2061 2062 2063 2064 2065 2066 2067 2068 2069 2070 2071 2072 2073 2074 2075 2076 2077 2078 2079 2080 2081 2082 2083 2084 2085 2086 2087 2088 2089 2090 2091 2092 2093 2094 2095 2096 2097 2098 2099 2100 2101 2102 2103 2104 2105 2106 2107 2108 2109 2110 2111 2112 2113 2114 2115 2116 2117 2118 2119 2120 2121 2122 2123 2124 2125 2126 2127 2128 2129 2130 2131 2132 2133 2134 2135 2136 2137 2138 2139 2140 2141 2142 2143 2144 2145 2146 2147 2148 2149 2150 2151 2152 2153 2154 2155 2156 2157 2158 2159 2160 2161 2162 2163 2164 2165 2166 2167 2168 2169 2170 2171 2172 2173 2174 2175 2176 2177 2178 2179 2180 2181 2182 2183 2184 2185 2186 2187 2188 2189 2190 2191 2192 2193 2194 2195 2196 2197 2198 2199 2200 2201 2202 2203 2204 2205 2206 2207 2208 2209 2210 2211 2212 2213 2214 2215 2216 2217 2218 2219 2220 2221 2222 2223 2224 2225 2226 2227 2228 2229 2230 2231 2232 2233 2234 2235 2236 2237 2238 2239 2240 2241 2242 2243 2244 2245 2246 2247 2248 2249 2250 2251 2252 2253 2254 2255 2256 2257 2258 2259 2260 2261 2262 2263 2264 2265 2266 2267 2268 2269 2270 2271 2272 2273 2274 2275 2276 2277 2278 2279 2280 2281 2282 2283 2284 2285 2286 2287 2288 2289 2290 2291 2292 2293 2294 2295 2296 2297 2298 2299 2300 2301 2302 2303 2304 2305 2306 2307 2308 2309 2310 2311 2312 2313 2314 2315 2316 2317 2318 2319 2320 2321 2322 2323 2324 2325 2326 2327 2328 2329 2330 2331 2332 2333 2334 2335 2336 2337 2338 2339 2340 2341 2342 2343 2344 2345 2346 2347 2348 2349 2350 2351 2352 2353 2354 2355 2356 2357 2358 2359 2360 2361 2362 2363 2364 2365 2366 2367 2368 2369 2370 2371 2372 2373 2374 2375 2376 2377 2378 2379 2380 2381 2382 2383 2384 2385 2386 2387 2388 2389 2390 2391 2392 2393 2394 2395 2396 2397 2398 2399 2400 2401 2402 2403 2404 2405 2406 2407 2408 2409 2410 2411 2412 2413 2414 2415 2416 2417 2418 2419 2420 2421 2422 2423 2424 2425 2426 2427 2428 2429 2430 2431 2432 2433 2434 2435 2436 2437 2438 2439 2440 2441 2442 2443 2444 2445 2446 2447 2448 2449 2450 2451 2452 2453 2454 2455 2456 2457 2458 2459 2460 2461 2462 2463 2464 2465 2466 2467 2468 2469 2470 2471 2472 2473 2474 2475 2476 2477 2478 2479 2480 2481 2482 2483 2484 2485 2486 2487 2488 2489 2490 2491 2492 2493 2494 2495 2496 2497 2498 2499 2500 2501 2502 2503 2504 2505 2506 2507 2508 2509 2510 2511 2512 2513 2514 2515 2516 2517 2518 2519 2520 2521 2522 2523 2524 2525 2526 2527 2528 2529 2530 2531 2532 2533 2534 2535 2536 2537 2538 2539 2540 2541 2542 2543 2544 2545 2546 2547 2548 2549 2550 2551 2552 2553 2554 2555 2556 2557 2558 2559 2560 2561 2562 2563 2564 2565 2566 2567 2568 2569 2570 2571 2572 2573 2574 2575 2576 2577 2578 2579 2580 2581 2582 2583 2584 2585 2586 2587 2588 2589 2590 2591 2592 2593 2594 2595 2596 2597 2598 2599 2600 2601 2602 2603 2604 2605 2606 2607 2608 2609 2610 2611 2612 2613 2614 2615 2616 2617 2618 2619 2620 2621 2622 2623 2624 2625 2626 2627 2628 2629 2630 2631 2632 2633 2634 2635 2636 2637 2638 2639 2640 2641 2642 2643 2644 2645 2646 2647 2648 2649 2650 2651 2652 2653 2654 2655 2656 2657 2658 2659 2660 2661 2662 2663 2664 2665 2666 2667 2668 2669 2670 2671 2672 2673 2674 2675 2676 2677 2678 2679 2680 2681 2682 2683 2684 2685 2686 2687 2688 2689 2690 2691 2692 2693 2694 2695 2696 2697 2698 2699 2700 2701 2702 2703 2704 2705 2706 2707 2708 2709 2710 2711 2712 2713 2714 2715 2716 2717 2718 2719 2720 2721 2722 2723 2724 2725 2726 2727 2728 2729 2730 2731 2732 2733 2734 2735 2736 2737 2738 2739 2740 2741 2742 2743 2744 2745 2746 2747 2748 2749 2750 2751 2752 2753 2754 2755 2756 2757 2758 2759 2760 2761 2762 2763 2764 2765 2766 2767 2768 2769 2770 2771 2772 2773 2774 2775 2776 2777 2778 2779 2780 2781 2782 2783 2784 2785 2786 2787 2788 2789 2790 2791 2792 2793 2794 2795 2796 2797 2798 2799 2800 2801 2802 2803 2804 2805 2806 2807 2808 2809 2810 2811 2812 2813 2814 2815 2816 2817 2818 2819 2820 2821 2822 2823 2824 2825 2826 2827 2828 2829 2830 2831 2832 2833 2834 2835 2836 2837 2838 2839 2840 2841 2842 2843 2844 2845 2846 2847 2848 2849 2850 2851 2852 2853 2854 2855 2856 2857 2858 2859 2860 2861 2862 2863 2864 2865 2866 2867 2868 2869 2870 2871 2872 2873 2874 2875 2876 2877 2878 2879 2880 2881 2882 2883 2884 2885 2886 2887 2888 2889 2890 2891 2892 2893 2894 2895 2896 2897 2898 2899 2900 2901 2902 2903 2904 2905 2906 2907 2908 2909 2910 2911 2912 2913 2914 2915 2916 2917 2918 2919 2920 2921 2922 2923 2924 2925 2926 2927 2928 2929 2930 2931 2932 2933 2934 2935 2936 2937 2938 2939 2940 2941 2942 2943 2944 2945 2946 2947 2948 2949 2950 2951 2952 2953 2954 2955 2956 2957 2958 2959 2960 2961 2962 2963 2964 2965 2966 2967 2968 2969 2970 2971 2972 2973 2974 2975 2976 2977 2978 2979 2980 2981 2982 2983 2984 2985 2986 2987 2988 2989 2990 2991 2992 2993 2994 2995 2996 2997 2998 2999 3000 3001 3002 3003 3004 3005 3006 3007 3008 3009 3010 3011 3012 3013 3014 3015 3016 3017 3018 3019 3020 3021 3022 3023 3024 3025 3026 3027 3028 3029 3030 3031 3032 3033 3034 3035 3036 3037 3038 3039 3040 3041 3042 3043 3044 3045 3046 3047 3048 3049 3050 3051 3052 3053 3054 3055 3056 3057 3058 3059 3060 3061 3062 3063 3064 3065 3066 3067 3068 3069 3070 3071 3072 3073 3074 3075 3076 3077 3078 3079 3080 3081 3082 3083 3084 3085 3086 3087 3088 3089 3090 3091 3092 3093 3094 3095 3096 3097 3098 3099 3100 3101 3102 3103 3104 3105 3106 3107 3108 3109 3110 3111 3112 3113 3114 3115 3116 3117 3118 3119 3120 3121 3122 3123 3124 3125 3126 3127 3128 3129 3130 3131 3132 3133 3134 3135 3136 3137 3138 3139 3140 3141 3142 3143 3144 3145 3146 3147 3148 3149 3150 3151 3152 3153 3154 3155 3156 3157 3158 3159 3160 3161 3162 3163 3164 3165 3166 3167 3168 3169 3170 3171 3172 3173 3174 3175 3176 3177 3178 3179 3180 3181 3182 3183 3184 3185 3186 3187 3188 3189 3190 3191 3192 3193 3194 3195 3196 3197 3198 3199 3200 3201 3202 3203 3204 3205 3206 3207 3208 3209 3210 3211 3212 3213 3214 3215 3216 3217 3218 3219 3220 3221 3222 3223 3224 3225 3226 3227 3228 3229 3230 3231 3232 3233 3234 3235 3236 3237 3238 3239 3240 3241 3242 3243 3244 3245 3246 3247 3248 3249 3250 3251 3252 3253 3254 3255 3256 3257 3258 3259 3260 3261 3262 3263 3264 3265 3266 3267 3268 3269 3270 3271 3272 3273 3274 3275 3276 3277 3278 3279 3280 3281 3282 3283 3284 3285 3286 3287 3288 3289 3290 3291 3292 3293 3294 3295 3296 3297 3298 3299 3300 3301 3302 3303 3304 3305 3306 3307 3308 3309 3310 3311 3312 3313 3314 3315 3316 3317 3318 3319 3320 3321 3322 3323 3324 3325 3326 3327 3328 3329 3330 3331 3332 3333 3334 3335 3336 3337 3338 3339 3340 3341 3342 3343 3344 3345 3346 3347 3348 3349 3350 3351 3352 3353 3354 3355 3356 3357 3358 3359 3360 3361 3362 3363 3364 3365 3366 3367 3368 3369 3370 3371 3372 3373 3374 3375 3376 3377 3378 3379 3380 3381 3382 3383 3384 3385 3386 3387 3388 3389 3390 3391 3392 3393 3394 3395 3396 3397 3398 3399 3400 3401 3402 3403 3404 3405 3406 3407 3408 3409 3410 3411 3412 3413 3414 3415 3416 3417 3418 3419 3420 3421 3422 3423 3424 3425 3426 3427 3428 3429 3430 3431 3432 3433 3434 3435 3436 3437 3438 3439 3440 3441 3442 3443 3444 3445 3446 3447 3448 3449 3450 3451 3452 3453 3454 3455 3456 3457 3458 3459 3460 3461 3462 3463 3464 3465 3466 3467 3468 3469 3470 3471 3472 3473 3474 3475 3476 3477 3478 3479 3480 3481 3482 3483 3484 3485 3486 3487 3488 3489 3490 3491 3492 3493 3494 3495 3496 3497 3498 3499 3500 3501 3502 3503 3504 3505 3506 3507 3508 3509 3510 3511 3512 3513 3514 3515 3516 3517 3518 3519 3520 3521 3522 3523 3524 3525 3526 3527 3528 3529 3530 3531 3532 3533 3534 3535 3536 3537 3538 3539 3540 3541 3542 3543 3544 3545 3546 3547 3548 3549 3550 3551 3552 3553 3554 3555 3556 3557 3558 3559 3560 3561 3562 3563 3564 3565 3566 3567 3568 3569 3570 3571 3572 3573 3574 3575 3576 3577 3578 3579 3580 3581 3582 3583 3584 3585 3586 3587 3588 3589 3590 3591 3592 3593 3594 3595 3596 3597 3598 3599 3600 3601 3602 3603 3604 3605 3606 3607 3608 3609 3610 3611 3612 3613 3614 3615 3616 3617 3618 3619 3620 3621 3622 3623 3624 3625 3626 3627 3628 3629 3630 3631 3632 3633 3634 3635 3636 3637 3638 3639 3640 3641 3642 3643 3644 3645 3646 3647 3648 3649 3650 3651 3652 3653 3654 3655 3656 3657 3658 3659 3660 3661 3662 3663 3664 3665 3666 3667 3668 3669 3670 3671 3672 3673 3674 3675 3676 3677 3678 3679 3680 3681 3682 3683 3684 3685 3686 3687 3688 3689 3690 3691 3692 3693 3694 3695 3696 3697 3698 3699 3700 3701 3702 3703 3704 3705 3706 3707 3708 3709 3710 3711 3712 3713 3714 3715 3716 3717 3718 3719 3720 3721 3722 3723 3724 3725 3726 3727 3728 3729 3730 3731 3732 3733 3734 3735 3736 3737 3738 3739 3740 3741 3742 3743 3744 3745 3746 3747 3748 3749 3750 3751 3752 3753 3754 3755 3756 3757 3758 3759 3760 3761 3762 3763 3764 3765 3766 3767 3768 3769 3770 3771 3772 3773 3774 3775 3776 3777 3778 3779 3780 3781 3782 3783 3784 3785 3786 3787 3788 3789 3790 3791 3792 3793 3794 3795 3796 3797 3798 3799 3800 3801 3802 3803 3804 3805 3806 3807 3808 3809 3810 3811 3812 3813 3814 3815 3816 3817 3818 3819 3820 3821 3822 3823 3824 3825 3826 3827 3828 3829 3830 3831 3832 3833 3834 3835 3836 3837 3838 3839 3840 3841 3842 3843 3844 3845 3846 3847 3848 3849 3850 3851 3852 3853 3854 3855 3856 3857 3858 3859 3860 3861 3862 3863 3864 3865 3866 3867 3868 3869 3870 3871 3872 3873 3874 3875 3876 3877 3878 3879 3880 3881 3882 3883 3884 3885 3886 3887 3888 3889 3890 3891 3892 3893 3894 3895 3896 3897 3898 3899 3900 3901 3902 3903 3904 3905 3906 3907 3908 3909 3910 3911 3912 3913 3914 3915 3916 3917 3918 3919 3920 3921 3922 3923 3924 3925 3926 3927 3928 3929 3930 3931 3932 3933 3934 3935 3936 3937 3938 3939 3940 3941 3942 3943 3944 3945 3946 3947 3948 3949 3950 3951 3952 3953 3954 3955 3956 3957 3958 3959 3960 3961 3962 3963 3964 3965 3966 3967 3968 3969 3970 3971 3972 3973 3974 3975 3976 3977 3978 3979 3980 3981 3982 3983 3984 3985 3986 3987 3988 3989 3990 3991 3992 3993 3994 3995 3996 3997 3998 3999 4000 4001 4002 4003 4004 4005 4006 4007 4008 4009 4010 4011 4012 4013 4014 4015 4016 4017 4018 4019 4020 4021 4022 4023 4024 4025 4026 4027 4028 4029 4030 4031 4032 4033 4034 4035 4036 4037 4038 4039 4040 4041 4042 4043 4044 4045 4046 4047 4048 4049 4050 4051 4052 4053 4054 4055 4056 4057 4058 4059 4060 4061 4062 4063 4064 4065 4066 4067 4068 4069 4070 4071 4072 4073 4074 4075 4076 4077 4078 4079 4080 4081 4082 4083 4084 4085 4086 4087 4088 4089 4090 4091 4092 4093 4094 4095 4096 4097 4098 4099 4100 4101 4102 4103 4104 4105 4106 4107 4108 4109 4110 4111 4112 4113 4114 4115 4116 4117 4118 4119 4120 4121 4122 4123 4124 4125 4126 4127 4128 4129 4130 4131 4132 4133 4134 4135 4136 4137 4138 4139 4140 4141 4142 4143 4144 4145 4146 4147 4148 4149 4150 4151 4152 4153 4154 4155 4156 4157 4158 4159 4160 4161 4162 4163 4164 4165 4166 4167 4168 4169 4170 4171 4172 4173 4174 4175 4176 4177 4178 4179 4180 4181 4182 4183 4184 4185 4186 4187 4188 4189 4190 4191 4192 4193 4194 4195 4196 4197 4198 4199 4200 4201 4202 4203 4204 4205 4206 4207 4208 4209 4210 4211 4212 4213 4214 4215 4216 4217 4218 4219 4220 4221 4222 4223 4224 4225 4226 4227 4228 4229 4230 4231 4232 4233 4234 4235 4236 4237 4238 4239 4240 4241 4242 4243 4244 4245 4246 4247 4248 4249 4250 4251 4252 4253 4254 4255 4256 4257 4258 4259 4260 4261 4262 4263 4264 4265 4266 4267 4268 4269 4270 4271 4272 4273 4274 4275 4276 4277 4278 4279 4280 4281 4282 4283 4284 4285 4286 4287 4288 4289 4290 4291 4292 4293 4294 4295 4296 4297 4298 4299 4300 4301 4302 4303 4304 4305 4306 4307 4308 4309 4310 4311 4312 4313 4314 4315 4316 4317 4318 4319 4320 4321 4322 4323 4324 4325 4326 4327 4328 4329 4330 4331 4332 4333 4334 4335 4336 4337 4338 4339 4340 4341 4342 4343 4344 4345 4346 4347 4348 4349 4350 4351 4352 4353 4354 4355 4356 4357 4358 4359 4360 4361 4362 4363 4364 4365 4366 4367 4368 4369 4370 4371 4372 4373 4374 4375 4376 4377 4378 4379 4380 4381 4382 4383 4384 4385 4386 4387 4388 4389 4390 4391 4392 4393 4394 4395 4396 4397 4398 4399 4400 4401 4402 4403 4404 4405 4406 4407 4408 4409 4410 4411 4412 4413 4414 4415 4416 4417 4418 4419 4420 4421 4422 4423 4424 4425 4426 4427 4428 4429 4430 4431 4432 4433 4434 4435 4436 4437 4438 4439 4440 4441 4442 4443 4444 4445 4446 4447 4448 4449 4450 4451 4452 4453 4454 4455 4456 4457 4458 4459 4460 4461 4462 4463 4464 4465 4466 4467 4468 4469 4470 4471 4472 4473 4474 4475 4476 4477 4478 4479 4480 4481 4482 4483 4484 4485 4486 4487 4488 4489 4490 4491 4492 4493 4494 4495 4496 4497 4498 4499 4500 4501 4502 4503 4504 4505 4506 4507 4508 4509 4510 4511 4512 4513 4514 4515 4516 4517 4518 4519 4520 4521 4522 4523 4524 4525 4526 4527 4528 4529 4530 4531 4532 4533 4534 4535 4536 4537 4538 4539 4540 4541 4542 4543 4544 4545 4546 4547 4548 4549 4550 4551 4552 4553 4554 4555 4556 4557 4558 4559 4560 4561 4562 4563 4564 4565 4566 4567 4568 4569 4570 4571 4572 4573 4574 4575 4576 4577 4578 4579 4580 4581 4582 4583 4584 4585 4586 4587 4588 4589 4590 4591 4592 4593 4594 4595 4596 4597 4598 4599 4600 4601 4602 4603 4604 4605 4606 4607 4608 4609 4610 4611 4612 4613 4614 4615 4616 4617 4618 4619 4620 4621 4622 4623 4624 4625 4626 4627 4628 4629 4630 4631 4632 4633 4634 4635 4636 4637 4638 4639 4640 4641 4642 4643 4644 4645 4646 4647 4648 4649 4650 4651 4652 4653 4654 4655 4656 4657 4658 4659 4660 4661 4662 4663 4664 4665 4666 4667 4668 4669 4670 4671 4672 4673 4674 4675 4676 4677 4678 4679 4680 4681 4682 4683 4684 4685 4686 4687 4688 4689 4690 4691 4692 4693 4694 4695 4696 4697 4698 4699 4700 4701 4702 4703 4704 4705 4706 4707 4708 4709 4710 4711 4712 4713 4714 4715 4716 4717 4718 4719 4720 4721 4722 4723 4724 4725 4726 4727 4728 4729 4730 4731 4732 4733 4734 4735 4736 4737 4738 4739 4740 4741 4742 4743 4744 4745 4746 4747 4748 4749 4750 4751 4752 4753 4754 4755 4756 4757 4758 4759 4760 4761 4762 4763 4764 4765 4766 4767 4768 4769 4770 4771 4772 4773 4774 4775 4776 4777 4778 4779 4780 4781 4782 4783 4784 4785 4786 4787 4788 4789 4790 4791 4792 4793 4794 4795 4796 4797 4798 4799 4800 4801 4802 4803 4804 4805 4806 4807 4808 4809 4810 4811 4812 4813 4814 4815 4816 4817 4818 4819 4820 4821 4822 4823 4824 4825 4826 4827 4828 4829 4830 4831 4832 4833 4834 4835 4836 4837 4838 4839 4840 4841 4842 4843 4844 4845 4846 4847 4848 4849 4850 4851 4852 4853 4854 4855 4856 4857 4858 4859 4860 4861 4862 4863 4864 4865 4866 4867 4868 4869 4870 4871 4872 4873 4874 4875 4876 4877 4878 4879 4880 4881 4882 4883 4884 4885 4886 4887 4888 4889 4890 4891 4892 4893 4894 4895 4896 4897 4898 4899 4900 4901 4902 4903 4904 4905 4906 4907 4908 4909 4910 4911 4912 4913 4914 4915 4916 4917 4918 4919 4920 4921 4922 4923 4924 4925 4926 4927 4928 4929 4930 4931 4932 4933 4934 4935 4936 4937 4938 4939 4940 4941 4942 4943 4944 4945 4946 4947 4948 4949 4950 4951 4952 4953 4954 4955 4956 4957 4958 4959 4960 4961 4962 4963 4964 4965 4966 4967 4968 4969 4970 4971 4972 4973 4974 4975 4976 4977 4978 4979 4980 4981 4982 4983 4984 4985 4986 4987 4988 4989 4990 4991 4992 4993 4994 4995 4996 4997 4998 4999 5000 5001 5002 5003 5004 5005 5006 5007 5008 5009 5010 5011 5012 5013 5014 5015 5016 5017 5018 5019 5020 5021 5022 5023 5024 5025 5026 5027 5028 5029 5030 5031 5032 5033 5034 5035 5036 5037 5038 5039 5040 5041 5042 5043 5044 5045 5046 5047 5048 5049 5050 5051 5052 5053 5054 5055 5056 5057 5058 5059 5060 5061 5062 5063 5064 5065 5066 5067 5068 5069 5070 5071 5072 5073 5074 5075 5076 5077 5078 5079 5080 5081 5082 5083 5084 5085 5086 5087 5088 5089 5090 5091 5092 5093 5094 5095 5096 5097 5098 5099 5100 5101 5102 5103 5104 5105 5106 5107 5108 5109 5110 5111 5112 5113 5114 5115 5116 5117 5118 5119 5120 5121 5122 5123 5124 5125 5126 5127 5128 5129 5130 5131 5132 5133 5134 5135 5136 5137 5138 5139 5140 5141 5142 5143 5144 5145 5146 5147 5148 5149 5150 5151 5152 5153 5154 5155 5156 5157 5158 5159 5160 5161 5162 5163 5164 5165 5166 5167 5168 5169 5170 5171 5172 5173 5174 5175 5176 5177 5178 5179 5180 5181 5182 5183 5184 5185 5186 5187 5188 5189 5190 5191 5192 5193 5194 5195 5196 5197 5198 5199 5200 5201 5202 5203 5204 5205 5206 5207 5208 5209 5210 5211 5212 5213 5214 5215 5216 5217 5218 5219 5220 5221 5222 5223 5224 5225 5226 5227 5228 5229 5230 5231 5232 5233 5234 5235 5236 5237 5238 5239 5240 5241 5242 5243 5244 5245 5246 5247 5248 5249 5250 5251 5252 5253 5254 5255 5256 5257 5258 5259 5260 5261 5262 | #!/usr/bin/env python3
"""Semantic checking: parse the generated code instead of pattern-matching it.
WHY THIS EXISTS
---------------
A violation_pattern is a regex over generated code. It catches textual
signatures, never meaning. Two of the corpus's critical rules were recorded in
NO_PATTERN_REASON as unreachable for exactly that reason:
odoo.constrains-must-list-every-field-read
odoo.stored-compute-needs-complete-depends
Both need the same thing: a set comparison between what a decorator declares and
what the method body actually reads. That is a tree walk, not a regex, and
Python's ast module makes it tractable without leaving the standard library.
WHAT A CHECKER OWES
-------------------
The same contract a violation_pattern owes, enforced by corpus.py validate: it
must fire on its rule's own 'wrong' sample and stay silent on its 'correct'
sample. A checker that catches nothing is useless; one that fires on good code
trains you to ignore it, which is worse.
HONEST SCOPE
------------
Python, and Odoo's XML data files, which expat reads with line numbers. There
is no JavaScript or TypeScript parser in the standard library, so the TS/JS
side gets balance_js -- a delimiter-balance scanner that is string-, comment-
and template-aware. That is a syntax smoke test, not a parser, and the layer
above reports UNKNOWN rather than PASS for everything it cannot see.
Stdlib only.
"""
import ast
import builtins
import collections
import importlib.util
import re
import textwrap
import xml.parsers.expat
import context
CHECKER_VERSION = "1.0.0"
# Attributes reached off a recordset that are ORM plumbing rather than fields.
# Deliberately short: every name added here is a field a checker will stop
# noticing, so the list earns entries only for names that can never be one.
ORM_ATTRS = frozenset([
"env", "ids", "id", "pool", "sudo", "with_context", "with_user", "browse",
"_name", "_table", "_cr", "_uid", "_context", "_origin", "_fields",
"_description", "_inherit", "_inherits", "_order", "_rec_name",
])
# ------------------------------------------------------------------ parsing
def parse_python(code):
"""Parse, then compile. Returns (tree, error) with at most one error set.
Both steps run because they fail on different things. ast.parse rejects bad
grammar; compile additionally rejects constructs that parse but cannot be
assembled -- return outside a function, await outside async def, duplicate
parameter names. Generated code hits the second class more often than the
first, because a model emits a plausible fragment lifted out of the context
that would have made it legal.
"""
try:
tree = ast.parse(code)
except SyntaxError as exc:
return None, _syntax_error(exc)
try:
compile(tree, "<answer>", "exec")
except (SyntaxError, ValueError, TypeError) as exc:
return tree, _syntax_error(exc)
return tree, None
def _syntax_error(exc):
return (getattr(exc, "lineno", None), getattr(exc, "msg", None) or str(exc))
Fragment = collections.namedtuple(
"Fragment", "tree error notes source line_offset")
# The four labels build_messages asks for. When they appear INSIDE the fence the
# block is not broken code, it is code followed by prose, and calling that a
# syntax error is the kind of false alarm that gets a checker switched off.
_LABEL_RE = re.compile(
r"^(Enforcement layer|Rules applied|Edge cases handled|Not handled)\s*:", re.I)
def parse_fragment(code):
"""Parse generated code that may be a fragment lifted out of a class.
WHY THIS IS NOT JUST parse_python: an Odoo answer is very often a method
shown at the indentation it had inside its model class, and a bare ast.parse
calls that 'unexpected indent'. Reporting a correct answer as uncompilable is
the worst failure a syntax layer has available -- it is wrong, it is loud,
and it masks every layer beneath it.
Three repairs are attempted, cheapest first, and each one that works is
RECORDED rather than hidden. The note is the finding: 'the labelled sections
were inside the code fence' is a real thing to tell the user, it just is not
a syntax error.
line_offset is how many lines were added in front of the original code, so a
caller reporting line numbers can subtract it and stay honest about where in
the answer the problem is.
"""
tree, error = parse_python(code)
if tree is not None and error is None:
return Fragment(tree, None, [], code, 0)
notes = []
candidate = code
lines = candidate.splitlines()
for index, line in enumerate(lines):
if _LABEL_RE.match(line.strip()):
candidate = "\n".join(lines[:index]).rstrip()
notes.append("the labelled answer sections were inside the code fence")
break
if notes:
tree, error = parse_python(candidate)
if tree is not None and error is None:
return Fragment(tree, None, notes, candidate, 0)
dedented = textwrap.dedent(candidate)
if dedented != candidate:
tree, error = parse_python(dedented)
if tree is not None and error is None:
notes.append("the block is indented: a method shown inside its class body")
return Fragment(tree, None, notes, dedented, 0)
candidate = dedented
# Ragged indentation dedent cannot fix -- a method whose decorator sits at
# one level and whose body sits at another. Give it a class to live in.
wrapped = "class _Fragment:\n" + "\n".join(
(" " + line if line.strip() else line) for line in candidate.splitlines())
tree, wrapped_error = parse_python(wrapped)
if tree is not None and wrapped_error is None:
notes.append("the block only parses as a class body fragment")
return Fragment(tree, None, notes, wrapped, 1)
return Fragment(None, error or wrapped_error, notes, code, 0)
# --------------------------------------------------------- path resolution
def _attr_path(node, aliases):
"""Resolve an attribute chain to a dotted field path relative to self.
rec.line_ids.price with rec bound to self yields ('line_ids', 'price').
Returns None when the base is not something known to be self-derived, which
is the honest answer for a name the checker never saw bound.
A chain may start from records picked by position. rec.line_ids[0].price and
rec.line_ids.sorted("sequence")[:1].price both yield ('line_ids', 'price'),
because indexing or slicing a recordset gives records of the same path.
Stopping at the subscript left the price unread, and a compute that takes
its value from the first line looked as if it read nothing from it.
A chain may start from a call, too, when the call hands back records of the
same path: rec.sudo().partner_id.name reads partner_id.name as surely as
rec.partner_id.name does. Which calls those are is _resolve_iterable's
list, the one a loop's iterable already goes through, so a loop and a read
cannot disagree about it (ARCHITECTURE 3.78).
"""
segments = []
cursor = node
while isinstance(cursor, ast.Attribute):
segments.append(cursor.attr)
cursor = cursor.value
if isinstance(cursor, ast.Subscript) and _indexes_records(cursor):
base = _resolve_iterable(cursor.value, aliases)
elif isinstance(cursor, ast.Call):
base = _resolve_iterable(cursor, aliases)
elif isinstance(cursor, ast.Name):
base = aliases.get(cursor.id)
else:
return None
if base is None:
return None
return base + tuple(reversed(segments))
def _indexes_records(subscript):
"""Does this subscript pick records by position: x[0], x[-1], x[1:]?
record[name] is the other thing a subscript on a recordset does, and it
reads the field called name, not records of the same path. So a subscript is
looked through only when its index is a literal integer or a slice.
rec.line_ids[i] could be either, and stays unresolved.
"""
index = subscript.slice
if isinstance(index, ast.Slice):
return True
if isinstance(index, ast.UnaryOp) and isinstance(index.op, ast.USub):
index = index.operand
return isinstance(index, ast.Constant) and type(index.value) is int
def _aliases(fn):
"""Names that stand for a recordset, and the path each reaches it by.
self is the root. 'for rec in self' makes rec the root as well. 'for line in
rec.line_ids' makes line the path ('line_ids',), which is what turns a later
line.price into the dependency line_ids.price.
Resolved to a fixpoint rather than in source order, because a comprehension
can bind a name from a loop variable introduced further down the tree and a
single forward pass would miss the second hop.
A name assigned records, pending = rec.review_ids.sorted("sequence")[:1],
stays unbound, and so does a loop over one. Binding it when every value
assigned to it resolves to one path is exact, and was measured: on odoo
core the depends check's reads through such names read 37 of 113 true,
28 of them smart defaults, the same ratio as the lambda binding
(ARCHITECTURE 3.72 and 3.80). Measured again on the tree facts, which
decide 27 of the 113, it still adds more false findings than true (3.87).
"""
aliases = {"self": ()}
for _ in range(6):
before = len(aliases)
for node in ast.walk(fn):
if isinstance(node, (ast.For, ast.AsyncFor, ast.comprehension)):
target, source = node.target, node.iter
else:
continue
if not isinstance(target, ast.Name):
continue # tuple unpacking: no single recordset to bind
path = _resolve_iterable(source, aliases)
if path is not None:
aliases[target.id] = path
if len(aliases) == before:
break
return aliases
def _resolve_iterable(node, aliases):
"""The recordset path an expression iterates over, if it is one.
Unwraps the calls that return a recordset of the same shape, and a subscript
by position, which picks records out of one. mapped() is excluded on
purpose: it returns values, not records, and treating it as a recordset
would invent dependencies that do not exist.
"""
if isinstance(node, ast.Name):
return aliases.get(node.id)
if isinstance(node, ast.Attribute):
return _attr_path(node, aliases)
if isinstance(node, ast.Subscript) and _indexes_records(node):
return _resolve_iterable(node.value, aliases)
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute):
if node.func.attr in ("filtered", "sorted", "exists", "with_context", "sudo"):
return _resolve_iterable(node.func.value, aliases)
return None
def _called_attrs(fn):
"""Attribute nodes sitting in a Call's function position.
rec.write(...) is a method, not a field. Without this every ORM call would
be reported as an undeclared field read, which is the fastest possible way
to make a checker worth ignoring.
"""
return {id(node.func) for node in ast.walk(fn)
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)}
def _message_only_attrs(fn):
"""Attribute nodes reachable only from inside a raise statement.
Found by crosscheck: the checker fired on the 'correct' sample of
odoo.constrains-iterate-recordset, which reads rec.display_name to build a
ValidationError 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 either -- demanding it in @api.constrains would be noise.
A field read in both the condition and the message is still caught, because
only the nodes inside the raise are excluded, not the field name.
A message assembled into a variable and raised on the next line is the same
read for the same reason, and the inline form was all this covered. That
missed the spelling OCA actually uses:
msg = self.env._("... %(journal)s ...", journal=journal.display_name)
raise ValidationError(msg)
which reported display_name as a field the constraint depends on.
Two more spellings came out of reading OpenSPP and odoo core on 2026-09-26.
A log line is a message too: `_logger.warning("... %s", rec.name)` reads a
field to phrase something, and five of OpenSPP's live false positives were
exactly that. And a message can be collected before it is raised --
`sold_out.append(_("... %(name)s", name=event.name))`, then
`raise ValidationError("\\n".join(sold_out))` -- which reads the same way as
the assignment form above.
"""
excluded = set()
raised = set()
for node in ast.walk(fn):
if isinstance(node, ast.Call) and _is_log_call(node):
for inner in ast.walk(node):
excluded.add(id(inner))
continue
if not isinstance(node, ast.Raise):
continue
for inner in ast.walk(node):
excluded.add(id(inner))
if isinstance(inner, ast.Name):
raised.add(inner.id)
if raised:
for node in ast.walk(fn):
if (isinstance(node, ast.Assign) and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)
and node.targets[0].id in raised):
for inner in ast.walk(node.value):
excluded.add(id(inner))
elif (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)
and node.func.attr in ("append", "extend")
and isinstance(node.func.value, ast.Name)
and node.func.value.id in raised):
for arg in node.args:
for inner in ast.walk(arg):
excluded.add(id(inner))
return excluded
LOG_METHODS = frozenset(["debug", "info", "warning", "warn", "error", "exception",
"critical", "log"])
def _is_log_call(call):
"""`_logger.warning(...)`, `logger.info(...)`, `logging.error(...)`."""
func = call.func
if not (isinstance(func, ast.Attribute) and func.attr in LOG_METHODS):
return False
owner = func.value
name = owner.id if isinstance(owner, ast.Name) else \
owner.attr if isinstance(owner, ast.Attribute) else ""
return name == "logging" or name.lower().lstrip("_").endswith(("logger", "log"))
def _field_reads(fn, aliases):
"""Every field path the body READS, as (path, lineno).
Store context is excluded: rec.total = ... is the compute writing its
result, and counting it would make every stored compute demand a dependency
on the field it produces.
"""
skip = _called_attrs(fn) | _message_only_attrs(fn)
reads = []
for node in ast.walk(fn):
if not isinstance(node, ast.Attribute) or id(node) in skip:
continue
if not isinstance(node.ctx, ast.Load):
continue
path = _attr_path(node, aliases)
# An Odoo field name never starts with an underscore -- that space is
# reserved for ORM internals and mixin configuration, and a field there
# would collide with them. ORM_ATTRS names the specific ones this code
# meets; the prefix is the general rule behind the list. Without it, a
# mixin reading self._time_window_overlap_check_field was told to add it
# to @api.constrains, where the ORM ignores it.
if path and path[0] not in ORM_ATTRS and not path[0].startswith("_"):
reads.append((path, node.lineno))
return reads
def _named_reads(fn, aliases):
"""Field paths a recordset method names in a string, as (path, lineno).
account.invoice_ids.mapped("amount_residual") reads invoice_ids.amount_residual
exactly as sum(inv.amount_residual for inv in account.invoice_ids) does, but
the only attribute access in it is invoice_ids. Read by attributes alone, the
first spelling looked fully declared by @api.depends("invoice_ids") and the
second did not, so the checker was silent on the more idiomatic of the two.
mapped() was the first of these (ARCHITECTURE 3.66). Its siblings name what
they read the same way: filtered("a.b") tests a.b on every record,
grouped("f") keys on f, sorted("f") orders on f, and filtered_domain() reads
the field of every leaf written out in its domain. _named_paths says which
paths each call names.
Only literals are followed. mapped(name) stays unread: what a variable
holds is a guess from here, and every guess in this checker is a finding
somebody has to read. A lambda stays unread too, and that one is not a
guess -- its parameter is each record of the receiver, exactly. It was
measured instead, and what the lambdas in these trees test is mostly not a
dependency (ARCHITECTURE 3.72, and again on the tree facts, 3.87). The
receiver resolves the way a loop's iterable does, through filtered(),
sorted(), sudo() and a subscript by position. A call inside a raise or a
log line is phrasing a message, as an attribute read there is.
"""
quiet = _message_only_attrs(fn)
reads = []
for node in ast.walk(fn):
if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)) \
or id(node) in quiet:
continue
named = _named_paths(node)
if not named:
continue
base = _resolve_iterable(node.func.value, aliases)
if base is None:
continue
for segments in named:
path = base + segments
if path[0] not in ORM_ATTRS and not path[0].startswith("_"):
reads.append((path, node.lineno))
return reads
# Recordset methods whose one argument can be a field path. mapped("a.b") and
# filtered("a.b") follow it through the relations; grouped("f") keys on f.
PATH_ARGUMENT_METHODS = frozenset(["mapped", "filtered", "grouped"])
def _named_paths(call):
"""The field paths, relative to its receiver, that a recordset call names."""
method = call.func.attr
if method in PATH_ARGUMENT_METHODS:
if len(call.args) != 1 or call.keywords:
return []
path = _literal_path(call.args[0])
return [path] if path else []
if method == "sorted":
keys = list(call.args[:1]) + [k.value for k in call.keywords if k.arg == "key"]
if len(keys) == 1 and isinstance(keys[0], ast.Constant) \
and isinstance(keys[0].value, str):
return _order_fields(keys[0].value)
return []
if method == "filtered_domain":
if len(call.args) == 1 and not call.keywords \
and isinstance(call.args[0], (ast.List, ast.Tuple)):
return _domain_fields(call.args[0])
return []
def _literal_path(node):
"""('a', 'b') for the string literal "a.b"; None for anything else."""
if not (isinstance(node, ast.Constant) and isinstance(node.value, str)):
return None
return _path_segments(node.value)
def _path_segments(text):
"""('a', 'b') for "a.b"; None when the text is not a dotted field path."""
segments = tuple(text.strip().split("."))
return segments if all(segment.isidentifier() for segment in segments) else None
ORDER_WORDS = frozenset(["asc", "desc", "nulls", "first", "last"])
def _order_fields(spec):
"""The field paths an order sorts on: "date desc, id" names date and id.
Odoo 18 takes one field name here, as itemgetter(key). Odoo 19 takes an
order: comma-separated terms, each a field with an optional .property and
:granularity, then asc or desc and nulls first or last
(_sorted_order_to_function in odoo/orm/models.py, 19.0). A spec with any
other word in it is read not at all rather than in part.
"""
paths = []
for term in spec.split(","):
words = term.split()
if not words or any(word.lower() not in ORDER_WORDS for word in words[1:]):
return []
path = _path_segments(words[0].split(":")[0])
if path is None:
return []
paths.append(path)
return paths
def _domain_fields(domain):
"""The field path of every leaf a domain literal writes out.
[("state", "=", "done"), ("partner_id.country_id", "in", ids)] names state and
partner_id.country_id. A leaf whose operator is any or not any holds a
domain on the related records, read through the leaf's own field:
("line_ids", "any", [("paid", "=", True)]) names line_ids and line_ids.paid.
"&", "|" and "!" name nothing, and neither does a leaf built at run time.
"""
paths = []
for leaf in domain.elts:
if not (isinstance(leaf, (ast.Tuple, ast.List)) and len(leaf.elts) == 3):
continue
field, operator, value = leaf.elts
path = _literal_path(field)
if path is None:
continue
paths.append(path)
if isinstance(operator, ast.Constant) and operator.value in ("any", "not any") \
and isinstance(value, (ast.List, ast.Tuple)):
paths.extend(path + inner for inner in _domain_fields(value))
return paths
# A decorator that is present and cannot be read statically is neither absent
# nor empty. @api.constrains(*FIELDS) used to collapse into "declares nothing",
# which made every field the body reads a missing declaration and phrased it
# "declares only nothing".
UNREADABLE = object()
def _note_unreadable(gaps, fn, decorator):
"""Record a method whose decorator could not be read. Not a finding.
Nothing is wrong with @api.constrains(*FIELDS); it is simply outside what a
static pass can decide. Reporting it as a violation is a false positive, and
reporting nothing at all is the silence this corpus exists to avoid -- so it
goes to the gap channel, where `check` prints it and no gate counts it.
Once per method, however many checkers meet it. Two of them read
@api.constrains, and the same unread method twice is one gap printed twice
by `check` and listed twice by fieldtest.
"""
if gaps is not None:
gap = ("{0}() has an @api.{1} whose arguments are not string literals, so "
"what it declares could not be read -- that method was not "
"checked".format(fn.name, decorator))
if gap not in gaps:
gaps.append(gap)
def _decorator(fn, module, name):
"""The @<module>.<name>(...) call decorating fn, or None.
One definition of which decorator that is, so a finding anchored on its line
and the arguments read from it cannot come from two different decorators.
"""
for dec in fn.decorator_list:
if not isinstance(dec, ast.Call):
continue
func = dec.func
if isinstance(func, ast.Attribute) and func.attr == name:
if isinstance(func.value, ast.Name) and func.value.id == module:
return dec
elif isinstance(func, ast.Name) and func.id == name:
return dec
return None
def _decorator_args(fn, module, name):
"""String literal arguments of @<module>.<name>(...).
Three answers, not two: None when the decorator is absent, UNREADABLE when
it is present and its arguments are not all string literals, and the list
otherwise. An empty list stays an empty list -- @api.constrains() really
does declare nothing, and that is readable.
"""
dec = _decorator(fn, module, name)
if dec is None:
return None
if any(not (isinstance(a, ast.Constant) and isinstance(a.value, str))
for a in dec.args):
return UNREADABLE
return [a.value for a in dec.args]
def _functions(tree):
return [n for n in ast.walk(tree)
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))]
# ------------------------------------------------------------- the checkers
def constrains_lists_every_field_read(code, gaps=None):
"""odoo.constrains-must-list-every-field-read
@api.constrains fires only when a listed field appears in the write vals. A
constraint that reads date_end while declaring only date_start never runs
when date_end alone is written, and the invalid record is stored in silence.
Only the first segment of each path is compared, because @api.constrains
silently ignores dotted names -- declaring partner_id.country_id registers
nothing, so a body reading it needs partner_id declared instead. That
asymmetry with @api.depends is its own rule
(odoo.constrains-ignores-dotted-names, constrains_declares_a_dotted_name
below); this checker only respects it.
Reads are the ones the depends check makes: attribute accesses, including
one after a subscript by position, and the fields a recordset method names
in a string (_named_reads). `if any(self.mapped("date_end"))` under
@api.constrains("date_start") was silent. Measured before it was taken: OCA
+1, OpenSPP 0, core +3, all read, 3 true. The false one filters on a
non-stored compute, which @api.constrains cannot name: a class this rule
already had for attribute reads (ARCHITECTURE 3.71).
That class is answered now, for a field the constraint's own class
declares. Odoo never runs a constraint for a field that is neither stored
nor inversible, and warns "@constrains parameter ... is not writeable" when
one is named, so telling the author to name it was advice that cannot be
taken. What can trigger the check is a write of what the field is computed
from: the roots of its compute's @api.depends, or the first field of its
related path, followed through any of those that is unwriteable too
(_unwriteable_inputs). A constraint that names them all is covered and
reported nothing; one that does not is told which of them it is missing.
When the inputs cannot be read, the finding says the field cannot be named
rather than asking for it (ARCHITECTURE 3.82).
"""
tree, _ = parse_python(code)
if tree is None:
return []
findings = []
for fn in _functions(tree):
declared = _decorator_args(fn, "api", "constrains")
if declared is None:
continue
if declared is UNREADABLE:
_note_unreadable(gaps, fn, "constrains")
continue
registered = {d.split(".")[0] for d in declared}
missing = {}
aliases = _aliases(fn)
for path, lineno in _field_reads(fn, aliases) + _named_reads(fn, aliases):
if path[0] not in registered:
missing.setdefault(path[0], lineno)
names = ", ".join(sorted(registered))
clause = "declares only {0}".format(names) if names else "declares nothing"
owner = _owner_of(tree, fn)
declarations = _class_fields(owner) if owner is not None else {}
for field, lineno in sorted(missing.items()):
if _writeable(declarations.get(field)) is not False:
findings.append((lineno,
"{0}() reads '{1}' but @api.constrains {2} -- "
"writing '{1}' alone skips the check entirely".format(
fn.name, field, clause)))
continue
inputs = _unwriteable_inputs(field, declarations, owner)
if inputs is None:
findings.append((lineno,
"{0}() reads '{1}', which is not stored, so no "
"write of '{1}' ever runs the check -- @api.constrains "
"{2}, and needs the fields '{1}' is computed from "
"instead".format(fn.name, field, clause)))
continue
lacking = sorted(inputs - registered)
if not lacking:
continue
findings.append((lineno,
"{0}() reads '{1}', which is not stored and is computed "
"from {2}, but @api.constrains {3} -- writing {4} alone "
"skips the check entirely".format(
fn.name, field, ", ".join(sorted(inputs)), clause,
" or ".join("'{0}'".format(name) for name in lacking))))
return findings
def _class_fields(cls):
"""{field name: {keyword: value node}} for the fields a class body declares.
Only `name = fields.Something(...)` statements directly in the body. A
field declared in another module, or by a mixin, is not here, and a caller
has to read that absence as unknown rather than as anything else.
"""
return {name: {k.arg: k.value for k in call.keywords if k.arg}
for name, call in _class_field_calls(cls).items()}
def _class_field_calls(cls):
"""{field name: the fields.Something(...) call declaring it}, as _class_fields."""
out = {}
for statement in cls.body:
if not (isinstance(statement, ast.Assign) and len(statement.targets) == 1
and isinstance(statement.targets[0], ast.Name)
and isinstance(statement.value, ast.Call)):
continue
func = statement.value.func
if not (isinstance(func, ast.Attribute) and isinstance(func.value, ast.Name)
and func.value.id == "fields"):
continue
out[statement.targets[0].id] = statement.value
return out
def _writeable(declaration):
"""Can a write of this field run a constraint naming it? True, False or None.
Odoo 18 runs a constraint when one of the names it declares is among the
fields written, and a field that is neither stored nor inversible never is:
_constraint_methods warns "@constrains parameter ... is not writeable" for
it, and that is all (odoo/models.py, 18.0 at 3c3e3b3). From the declaration,
per _get_attrs and setup_related in odoo/fields.py at the same commit:
compute= not stored unless store=True; inversible with an inverse=
related= not stored unless store=True; readonly unless readonly=False,
and inversible only when it is not readonly
neither a plain field, stored
None when the declaration is absent or says it with something other than a
literal: unknown, which is never read as not writeable.
"""
if declaration is None:
return None
if "compute" not in declaration and "related" not in declaration:
return True
store = declaration.get("store")
if store is not None:
if not isinstance(store, ast.Constant):
return None
if store.value:
return True
if "compute" in declaration:
return "inverse" in declaration
readonly = declaration.get("readonly")
if readonly is None:
return False
if not isinstance(readonly, ast.Constant):
return None
return not readonly.value
def _unwriteable_inputs(field, declarations, cls, seen=None):
"""The writeable fields an unwriteable field is computed from, or None.
The roots of its compute's @api.depends, or the first field of its related
path: @api.constrains names fields of its own model only, so a dependency
through a relation is reached by naming the relation, as a read through one
is. A root that is itself unwriteable is followed to what it is computed
from. None when any step cannot be read: a compute method not in this class,
@api.depends absent or not literal, a related path that is not a string.
`id` never changes and is left out.
"""
seen = set(seen or ()) | {field}
declaration = declarations[field]
related = declaration.get("related")
if related is not None:
if not (isinstance(related, ast.Constant) and isinstance(related.value, str)):
return None
roots = {related.value.split(".")[0]}
else:
compute = declaration["compute"]
if isinstance(compute, ast.Constant) and isinstance(compute.value, str):
name = compute.value
elif isinstance(compute, ast.Name):
name = compute.id
else:
return None
method = next((node for node in cls.body
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
and node.name == name), None)
if method is None:
return None
depends = _decorator_args(method, "api", "depends")
if depends is None or depends is UNREADABLE:
return None
roots = {path.split(".")[0] for path in depends}
inputs = set()
for root in roots - {"id"}:
if root in seen:
continue
if _writeable(declarations.get(root)) is False:
deeper = _unwriteable_inputs(root, declarations, cls, seen)
if deeper is None:
return None
inputs |= deeper
else:
inputs.add(root)
return inputs
def constrains_declares_a_dotted_name(code, gaps=None):
"""odoo.constrains-ignores-dotted-names
@api.constrains registers plain field names. A dotted one such as
partner_id.country_id is accepted and the module installs, but it can never
be among the written fields _validate_fields compares the declaration with,
so the ORM never runs the check for it. Odoo 18 logs "@constrains parameter
... is not a field name" the first time the model collects its constraints,
and that is all (odoo/models.py, 18.0 at 3c3e3b3).
Was a regex for a quoted name of two segments in lowercase letters and
underscores. partner_id.country_id.code has three, so where the regex wanted
the closing quote there was a dot; and the class had no digits, so a name
through any l10n_ field was invisible to it. _decorator_args reads the
arguments exactly, and a computed list -- @api.constrains(*FIELDS), or the
callable form Odoo also accepts -- goes to the gap channel rather than being
guessed at.
One finding per method, on the decorator line, where the names are and where
a marker goes. The message says what is left to trigger the check: nothing
when every name is dotted, and only the plain names when some are.
0 dotted names in 815 @api.constrains across odoo core, OCA and OpenSPP, so
this moves nothing measured there. It is for code those trees do not hold,
and for generated answers, where @api.depends beside it takes the same
dotted name and means it. ARCHITECTURE 3.70.
"""
tree, _ = parse_python(code)
if tree is None:
return []
findings = []
for fn in _functions(tree):
declared = _decorator_args(fn, "api", "constrains")
if declared is None:
continue
if declared is UNREADABLE:
_note_unreadable(gaps, fn, "constrains")
continue
dotted = [name for name in declared if "." in name]
if not dotted:
continue
plain = [name for name in declared if "." not in name]
effect = ("the ORM runs the check only when {0} is written".format(
" or ".join(plain)) if plain else "the ORM never runs the check")
findings.append((
_decorator(fn, "api", "constrains").lineno,
"{0}() declares {1} in @api.constrains, which ignores dotted names "
"-- {2}".format(fn.name, ", ".join("'{0}'".format(name)
for name in dotted), effect)))
return findings
def _declared_covers(read, declared):
"""Is a field read already covered by something @api.depends declares?
Exactly, or by a longer path that runs through it. @api.depends('a.b')
registers the whole chain, so the ORM recomputes when a changes as well as
when a.b does, and a body reading rec.a needs nothing further. Demanding the
prefix be spelled out again reported correct code as stale: 42 of this
checker's 104 hits on OpenSPP were exactly that, 40% of its output.
The converse is the rule itself and must not follow: a declared 'a' does
NOT cover a read of a.b, which is the stale value this exists to find.
"""
return any(d == read or d.startswith(read + ".") for d in declared)
# Fields the ORM adds to every model itself, with a declaration this corpus
# can read and a default of NOT stored. A model that wants one of these in the
# database says so in its own file, and then the ordinary storedness test sees
# it. Only display_name is here: the other automatic fields -- id, create_uid,
# create_date, write_uid, write_date -- are never computed by module code, so
# listing them would be describing a shape nobody writes.
FRAMEWORK_UNSTORED = frozenset(["display_name"])
def depends_covers_compute_reads(code, facts=None, gaps=None):
"""odoo.stored-compute-needs-complete-depends
A stored compute recomputes only when a declared dependency changes. A body
reading line_ids.price while declaring only line_ids keeps a stale total in
the database after a price edit, and every report built on that column is
quietly wrong from then on.
Full dotted paths are compared, not roots: Odoo needs line_ids.price spelled
out, and line_ids alone does not imply it.
That asymmetry runs one way only, and reading it as symmetric was this
checker's largest source of false positives. A declared line_ids.price DOES
cover a body that reads rec.line_ids -- the dependency is on the whole
chain, so the ORM already recomputes when the relation itself changes. See
_declared_covers.
Two things are NOT reported, both learned by running this over 35k lines of
OCA code rather than over the corpus's own samples:
A path ending in .id. An id cannot change without the relation changing,
so a read of journal_id.account_id.id is covered by journal_id.account_id,
and one that is not declared is reported under that name. Odoo would take
the .id spelling without a word: id is a field of every model, and
resolve_depends keeps the relation it ends on and adds a trigger on id,
which fires when a related record is deleted (odoo/models.py,
odoo/fields.py, 18.0 at 3c3e3b3). Reporting it asked for a longer
spelling of what was declared already. 13 of 121 findings on OCA were
this.
A read of a field this same method COMPUTES. Reading your own output
mid-compute is usually a bug, but the fix is never "declare a dependency
on it": Odoo drops a field from its own @api.depends without a word.
resolve_depends skips it ("do not make self trigger itself",
odoo/fields.py, 18.0 at 3c3e3b3), and every recompute and onchange runs
off the triggers it builds, so the entry does nothing. Emitting advice
that cannot be followed is worse than saying nothing, and there is no
rule in the corpus about self-reads for this to become. 48 of 121
findings on OCA were reads of a computed field, most of them the
method's own.
A path through the output is exempt too, and there the premise is
thinner. Odoo skips only the field's own name, so @api.depends("total.x")
is kept, and the compute re-runs when x changes on the record total
points to. Measured with the exemption narrowed to the bare field
(ARCHITECTURE 3.73): OCA and OpenSPP 0, core +37, all read. 17 true, 16
of them a smart default checking its own earlier choice against
configuration elsewhere; 20 false, where what total points to cannot
change or write() refuses the change. So the exemption stays whole.
What a method computes is what it assigns and what the file declares
with compute="<that method>" (_computed_fields). A helper handed the
records and the field name, update() and write() fill a field with no
attribute store to see, and that field used to read as a dependency of
the method producing it: calendar.recurrence's _compute_rrule on core,
and the two OpenSPP findings already classified false as self-read.
Core 2,358 -> 2,357, OpenSPP 163 -> 161, nothing added (ARCHITECTURE
3.72).
And a third, added after classifying the survivors: a compute whose field is
declared in this same file WITHOUT store=True. The rule is about STORED
computes -- "written once and never recomputed... survives restarts" -- and
none of that happens to a value the database never holds. 30 of 95 findings
on OCA were non-stored computes, outside the rule's own scope.
That test is deliberately conditional. Most of this checker's work is on
generated snippets, where no field declaration is present at all -- the
rule's own wrong sample is a bare @api.depends and a method body. So the
file's evidence is used when it exists and the rule's assumption stands when
it does not: proven-not-stored is dropped, unknown stays a finding.
Reads are attribute accesses plus the fields a recordset method names in a
string (_named_reads), and every exemption above applies to both. mapped()
came first: sum(rec.invoice_ids.mapped("amount_residual")) under a bare
@api.depends("invoice_ids") was silent while the same read spelled as a
generator was reported. Core 2,336 -> 2,345; all 9 read, 8 true. filtered(),
sorted(), grouped() and filtered_domain() followed, with a read after a
subscript by position (rec.line_ids[0].amount, _attr_path): OCA +1, OpenSPP
+6, core +17, all 24 read, 15 true. The 9 false are classes this checker
already had for attribute reads (ARCHITECTURE 3.71).
Three answers come from the tree when one was walked, each only where the
module giving it is installed with this one (ARCHITECTURE 3.83):
A compute whose fields every declaration in a complete walk leaves
unstored is outside the rule, as one declared so in this file always
was (_unstored_in_tree).
Odoo merges @api.depends from every definition of the method in the
model's MRO, so a dependency another definition declares is declared
(_merge_into_declared). Only from the modules this one depends on: a
sibling that overrides the same compute is not always installed.
Odoo's triggers are transitive, through stored and unstored fields
alike (get_field_trigger_tree, odoo/modules/registry.py at 3c3e3b3).
A declared field that is computed or related covers what it depends on
in turn (_covered_transitively).
"""
tree, _ = parse_python(code)
if tree is None:
return []
storedness = _compute_storedness(tree)
computed = _computed_fields(tree)
closure = context.module_closure(facts, (facts or {}).get("module"))
findings = []
for fn in _functions(tree):
declared = _decorator_args(fn, "api", "depends")
if declared is None:
continue
if declared is UNREADABLE:
_note_unreadable(gaps, fn, "depends")
continue
if storedness.get(fn.name) is False:
continue
owner = _owner_of(tree, fn)
if owner is not None and _is_transient(owner):
# A wizard's stored compute is written and read inside one
# interaction. "Written once and never recomputed, survives
# restarts, diverges from reality" is about a row that outlives the
# thing it was computed from, and this one does not.
continue
model = _class_model(owner)[0] if owner is not None else None
if storedness.get(fn.name) is None and _unstored_in_tree(facts, model, fn.name):
continue
registered = set(declared) | _merge_into_declared(facts, closure, model, fn.name)
produced = _assigned_attrs(fn)
if produced and produced <= FRAMEWORK_UNSTORED and \
storedness.get(fn.name) is None:
# A field the framework adds to every model, which this file does
# not redeclare. "Unknown storedness" is the right reading for an
# ordinary field declared in another module; it is the wrong one
# here, because the framework's own declaration is visible and
# says not stored. odoo/models.py adds display_name as
# `fields.Char(automatic=True, compute='_compute_display_name',
# search=...)` with no store, so an override that omits a
# dependency cannot leave a stale row: there is no row.
#
# 121 of the 1051 findings this rule made on odoo core were
# _compute_display_name overrides of exactly that shape. The
# narrowing costs nothing measurable: all 10 hand-verified true
# positives that compute display_name -- every one of them in
# OpenSPP -- declare `store=True` on it in their own file, so
# storedness reads True for them and they are untouched.
continue
missing = {}
aliases = _aliases(fn)
own = produced | computed.get(fn.name, set())
for path, lineno in _field_reads(fn, aliases) + _named_reads(fn, aliases):
while len(path) > 1 and path[-1] in ORM_ATTRS:
path = path[:-1]
if path[0] in own:
continue
dotted = ".".join(path)
if _declared_covers(dotted, registered):
continue
if not _covered_transitively(facts, closure, model, registered, dotted):
missing.setdefault(dotted, lineno)
if missing:
# `nexa check` reads the modules this one depends on only now, for a
# read the module's own declarations leave uncovered; most files never
# have one (ARCHITECTURE 3.86). More declarations can only cover more,
# so the reads left are the only ones to ask about again.
wider = context.with_dependencies(facts)
if wider is not None:
reach = context.module_closure(wider, wider.get("module"))
merged = registered | _merge_into_declared(wider, reach, model, fn.name)
missing = {dotted: lineno for dotted, lineno in missing.items()
if not _declared_covers(dotted, merged)
and not _covered_transitively(wider, reach, model, merged, dotted)}
for dotted, lineno in sorted(missing.items()):
findings.append((lineno,
"{0}() reads '{1}' but @api.depends does not declare it -- "
"the stored value goes stale when '{1}' changes".format(
fn.name, dotted)))
return findings
def _class_model(cls):
"""(model, named, inherited) for a class, read as Odoo reads it.
`_name`, or failing that a single `_inherit`. Odoo names a class that
inherits several models without a `_name` after the class itself
(odoo/models.py, 18.0 at 3c3e3b3); that is no model the tree declares, so
it is None here.
"""
name = inherit = None
for node in cls.body:
if not (isinstance(node, ast.Assign) and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)):
continue
if node.targets[0].id == "_name":
name = _literal_string(node.value)
elif node.targets[0].id == "_inherit":
inherit = _literal_strings(node.value)
inherit = inherit or []
return (name or (inherit[0] if len(inherit) == 1 else None), name is not None,
inherit)
def _lineage(facts, model, closure):
"""The model and every model it inherits, transitively, by the inheritance
the modules in `closure` declare, or every module's when closure is None."""
parents = facts.get("orm_parents") or {}
seen, stack = {model}, [model]
while stack:
for module, parent in parents.get(stack.pop(), ()):
if (closure is None or module in closure) and parent not in seen:
seen.add(parent)
stack.append(parent)
return seen
def _unstored_in_tree(facts, model, method):
"""Does every declaration the walk holds leave this compute's fields unstored?
Only from a complete walk that names the model and everything it inherits:
"nothing stores it" is true only of everything read, and an extension
cannot see the declaration that created the field. Adding compute= to a
field without store=True makes it unstored, whatever an earlier
declaration of it said (_get_attrs, odoo/fields.py at 3c3e3b3), so one
store=True anywhere is what keeps it stored.
"""
if not (facts and facts.get("walked") and facts.get("complete") and model):
return False
named = facts.get("orm_named") or set()
lineage = _lineage(facts, model, None)
if any(m not in named for m in lineage):
return False
declared = facts.get("orm_fields") or {}
found = False
for field in {f for m in lineage for f, rows in declared.get(m, {}).items()
if any(d["compute"] == method for _, d in rows)}:
found = True
stores = [d["store"] for m in lineage for _, d in declared.get(m, {}).get(field, ())]
if any(store is True or store == "?" for store in stores):
return False
return found
def _merged_depends(facts, closure, model, method):
"""The @api.depends of every definition of `method` in the model's MRO
that a module in `closure` declares."""
if not facts or closure is None or not model:
return set()
methods = facts.get("orm_methods") or {}
merged = set()
for m in _lineage(facts, model, closure):
for module, depends in methods.get(m, {}).get(method, ()):
if module in closure and depends:
merged.update(depends)
return merged
def _merge_into_declared(facts, closure, model, method):
"""What Odoo adds to this method's own @api.depends from its other
definitions: the depends check's use of _merged_depends."""
return _merged_depends(facts, closure, model, method)
def _covered_transitively(facts, closure, model, declared, read):
"""Does a declared dependency reach `read` through what the fields on its
way depend on?
Each step of a declared path is a field of the model the path has reached
so far. A computed one depends on its compute's merged @api.depends and a
related one on its path, both relative to that model, so a step replaces
the rest of the path with the field's own dependency and keeps the prefix
that reached it. A relational field moves the next step to its comodel; a
related one stops there, since its comodel is not in its declaration. Only
declarations from modules in `closure` count.
The search goes toward the read. A step keeps its prefix, so a step taken
where the path has already left the read's can never come back to it, and
is not taken. Every path met is then a prefix of the read followed by one
dependency some field declares, a finite set, so the search ends without a
limit, and its answer does not depend on the order it looks in.
"""
if not facts or closure is None or not model:
return False
target = tuple(read.split("."))
queue = collections.deque(sorted(tuple(path.split(".")) for path in declared))
seen = set(queue)
while queue:
path = queue.popleft()
if path[:len(target)] == target:
return True
current = model
for index in range(min(len(path), len(target))):
rows = _declarations(facts, closure, current, path[index])
for row in rows:
for dependency in _field_dependencies(facts, closure, current, row):
reached = path[:index] + tuple(dependency.split("."))
if reached not in seen:
seen.add(reached)
queue.append(reached)
if path[index] != target[index]:
break
comodel = next((row["comodel"] for row in rows if row["comodel"]), None)
if comodel is None:
break
current = comodel
return False
def _declarations(facts, closure, model, field):
"""Every declaration of `field` on the model's lineage from a module in
`closure`, in a fixed order."""
fields = facts.get("orm_fields") or {}
return [row for m in sorted(_lineage(facts, model, closure))
for module, row in fields.get(m, {}).get(field, ())
if module in closure]
def _field_dependencies(facts, closure, model, row):
"""What one declaration of a field makes it depend on, relative to its model."""
if row["related"]:
return [row["related"]]
if row["compute"]:
return sorted(_merged_depends(facts, closure, model, row["compute"]))
return []
def _test_atoms(fn):
"""Every condition tested by an if or while in this function, with and/or
flattened. `a and (b or c)` yields a, b, c."""
atoms = []
stack = [node.test for node in ast.walk(fn)
if isinstance(node, (ast.If, ast.While))]
while stack:
node = stack.pop()
if isinstance(node, ast.BoolOp):
stack.extend(node.values)
else:
atoms.append(node)
return atoms
def _is_literal_comparison(node):
"""A comparison of something against a literal, and nothing more.
Deliberately narrow: this decides whether a constraint is doing ONLY what a
CHECK can do, so anything it does not recognise has to count against the
claim, never for it.
"""
if not isinstance(node, ast.Compare):
return False
operands = [node.left] + list(node.comparators)
return any(isinstance(o, ast.Constant) and not isinstance(o.value, bool)
and isinstance(o.value, (int, float, str)) for o in operands)
def _compute_storedness(tree):
"""{compute method name: True|False} from the field declarations in scope.
A name absent from this map is unknown, not unstored: the declaration may
live in another file, or the code may be a fragment with no declaration at
all. Callers must treat absence and False differently.
"""
stored = {}
for node in ast.walk(tree):
if not isinstance(node, ast.Call) or not isinstance(node.func, ast.Attribute):
continue
compute = store = None
for keyword in node.keywords:
if keyword.arg == "compute" and isinstance(keyword.value, ast.Constant):
compute = keyword.value.value
elif keyword.arg == "store" and isinstance(keyword.value, ast.Constant):
store = keyword.value.value
if isinstance(compute, str):
# A field extended in two places counts as stored if either stores
# it: one declaration that writes to the database is enough for the
# value to go stale in it.
stored[compute] = bool(store) or bool(stored.get(compute))
return stored
def _computed_fields(tree):
"""{compute method name: the field names this file declares it computes}.
`account_type = fields.Selection(compute="_compute_account_type", ...)`
maps _compute_account_type to {"account_type"}. That is the method's output
whether or not the body assigns it where _assigned_attrs can see: a helper
handed the records and the field name, update({...}), write({...}) and
rec[name] = ... all fill it without an attribute store.
"""
out = collections.defaultdict(set)
for node in ast.walk(tree):
if not (isinstance(node, ast.Assign) and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)
and isinstance(node.value, ast.Call)):
continue
for keyword in node.value.keywords:
if (keyword.arg == "compute" and isinstance(keyword.value, ast.Constant)
and isinstance(keyword.value.value, str)):
out[keyword.value.value].add(node.targets[0].id)
return out
def _assigned_attrs(fn):
"""Attribute names this function writes: rec.total = ... -> {'total'}.
These are the compute's outputs. _field_reads already drops the assignment
itself, but a body that writes rec.total and elsewhere READS rec.total.name
would otherwise be told to depend on the field it is producing.
"""
names = set()
for node in ast.walk(fn):
targets = []
if isinstance(node, ast.Assign):
targets = node.targets
elif isinstance(node, (ast.AugAssign, ast.AnnAssign)):
targets = [node.target]
for target in targets:
for inner in ast.walk(target):
if isinstance(inner, ast.Attribute) and isinstance(inner.ctx, ast.Store):
names.add(inner.attr)
return names
_SQL_COLUMNS = re.compile(r"\b(?:UNIQUE|CHECK)\s*\(([^)]*)\)", re.I)
def _searches_self(call):
"""Is this search on THIS model, rather than on another one?
`self.search(...)` and `self.sudo().search(...)` are uniqueness on this
table, which is what a UNIQUE entry replaces. `self.env["other"].search(...)`
is uniqueness across tables, which no constraint on this table can express.
"""
receiver = call.func.value
while isinstance(receiver, ast.Call) and isinstance(receiver.func, ast.Attribute):
receiver = receiver.func.value
return isinstance(receiver, ast.Name) and receiver.id == "self"
def _owner_of(tree, fn):
"""The ClassDef a function is defined in, or None for a module-level one."""
for node in ast.walk(tree):
if isinstance(node, ast.ClassDef) and any(child is fn for child in node.body):
return node
return None
def _columns_already_constrained(tree, series=None):
"""{class node id: column names a SQL constraint on that class already covers}
Two spellings, and neither works everywhere.
`_code_unique = models.Constraint("UNIQUE(code)", msg)` is the 19 one: odoo
18.0 core has no models.Constraint at all and declares _sql_constraints in
174 files. `_sql_constraints = [(name, "UNIQUE(code)", msg)]` is the classic
one and is IGNORED from 19: the registry logs a warning at load and the
constraint is never created.
So on a module that pins 19 or later, a _sql_constraints block is not a
column being spoken for -- it is a column that nobody is watching and
everybody thinks somebody is. Reading it as satisfaction made this checker
silent on exactly the file that carries the bug: OpenSPP's
spp.cr.type.reason.document declared (cr_type_id, reason) that way,
duplicates accumulated unnoticed from 19.0.3.0.0, and the fix (208d9758)
re-declared it as models.Constraint. This checker said nothing either side
of that commit.
`series` unknown keeps the old reading. Firing on every 16, 17 and 18 module
that did exactly the right thing is a worse trade than missing the 19 case,
and `check` already reports the absent manifest as a gap.
Column names are read out of the SQL text, which is crude and sufficient:
the question is only whether this column is already spoken for.
"""
dead_legacy = _series_at_least(series, 19.0)
out = {}
for node in ast.walk(tree):
if not isinstance(node, ast.ClassDef):
continue
columns = set()
for statement in node.body:
if not isinstance(statement, ast.Assign):
continue
if dead_legacy and _assigns_name(statement, "_sql_constraints"):
continue
for literal in ast.walk(statement):
if isinstance(literal, ast.Constant) and isinstance(literal.value, str):
for group in _SQL_COLUMNS.findall(literal.value):
columns.update(
re.findall(r"[A-Za-z_][A-Za-z0-9_]*", group))
out[id(node)] = columns
return out
def _assigns_name(statement, name):
return any(isinstance(t, ast.Name) and t.id == name for t in statement.targets)
def legacy_sql_constraints_on_19(code, facts=None):
"""odoo.sql-constraints-are-ignored-from-19
A class-body `_sql_constraints` assignment that Odoo 19 will not read. From
19 the registry logs one warning at load and creates nothing, so every
UNIQUE and CHECK in the block is a constraint nobody is enforcing and
everybody thinks somebody is.
The incident is OpenSPP's 208d9758 (#394): spp.cr.type.reason.document
declared (cr_type_id, reason) this way, duplicates were saved from
19.0.3.0.0, and the fix re-declared it as models.Constraint and added a
lint of its own because nothing in CI had caught it. The rest of that tree
had already moved, 72 files of models.Constraint to this one holdout.
Which series, three answers:
19 or later reported.
16 to 18 silent. _sql_constraints is the ONLY spelling there --
odoo 18.0 core declares it in 174 files and has no
models.Constraint -- so a finding would be advice that
cannot be taken. OCA's twenty are all 18.0.
unknown silent when a tree was walked and does not pin a series,
which is odoo core itself: its manifests carry versions
like "1.0", and 174 findings there would all be wrong.
Reported, advisory, when NOTHING was walked -- a bare
snippet, validate's own samples -- because "nobody looked"
and "looked and it does not say" are different answers,
and silence on the first would hide the rule entirely.
An empty list declares nothing and is not reported.
"""
facts = facts or {}
series = facts.get("odoo_series")
if _series_at_least(series, 19.0):
grade = None
elif series is None and not facts.get("walked"):
grade = UNPROVEN
else:
return []
tree, _ = parse_python(code)
if tree is None:
return []
findings = []
for cls in ast.walk(tree):
if not isinstance(cls, ast.ClassDef):
continue
for statement in cls.body:
if isinstance(statement, ast.Assign):
targets, value = statement.targets, statement.value
elif isinstance(statement, ast.AnnAssign) and statement.value is not None:
targets, value = [statement.target], statement.value
else:
continue
if not any(isinstance(t, ast.Name) and t.id == "_sql_constraints"
for t in targets):
continue
if isinstance(value, (ast.List, ast.Tuple)) and not value.elts:
continue
findings.append((
statement.lineno,
"{0} declares _sql_constraints, which Odoo 19 ignores{1}: the "
"registry logs a warning and creates no constraint, so nothing "
"enforces it -- declare each one as models.Constraint".format(
cls.name, "" if grade is None else
" (the series is unknown here; this holds from 19)"),
grade))
return findings
def _series_at_least(series, floor):
"""Is this module pinned to `floor` or later? False when nothing said."""
try:
return float(series) >= floor
except (TypeError, ValueError):
return False
def prefer_sql_constraint_applies(code, facts=None):
"""odoo.prefer-sql-constraint
Fires on an @api.constrains that PostgreSQL could have enforced itself.
Two shapes qualify, and deliberately nothing else:
uniqueness the body calls search or search_count. That is a uniqueness
test with a race inside it -- two concurrent transactions
both find nothing, both insert, and the duplicate lands.
UNIQUE does not race.
bound the body does nothing but compare its own declared columns
against literals. CHECK(qty > 0) holds for every writer
forever, including the ones that never load the ORM.
NOT covered, and this is now a settled decision rather than an omission.
CHECK(date_end >= date_start) is perfectly good SQL, so the obvious reading
is that recall was left on the table. Two things were weighed, and widening
lost on both:
It would fire on another rule's correct sample. The canonical two-field
constraint in this corpus is odoo.constrains-must-list-every-field-read's
own fix -- constrains('date_start', 'date_end') guarding date_end <
date_start. A widened checker flags it, `nexa crosscheck` reports a
standing hit forever, and the two rules appear to disagree when they do
not. A correct sample that another checker condemns is a corpus defect;
that is exactly why odoo.constrains-iterate-recordset's sample was
rewritten rather than exempted.
The suggested replacement is weaker than what it replaces. PostgreSQL
treats a CHECK as satisfied when its expression evaluates to NULL, so
CHECK(date_end >= date_start) enforces nothing on a row where either date
is still empty -- which, for two optional columns, is most rows for most
of their life. The Python constraint it would replace usually carries an
explicit `if rec.date_start and rec.date_end` guard saying the same thing
on purpose. Advising the swap without that caveat reads as a
tightening and is not one. A single-column bound does not have the same
problem in practice: a numeric column with a default is rarely NULL.
So the gap stays, pinned by
tests/test_semantics.py::test_two_column_comparison_is_silent. Reopening it
means answering both points, not just observing that the SQL is valid.
Out of scope by definition: anything reaching a related record, calling a
method, or touching self.env. A CHECK genuinely cannot express those, which
is what odoo.check-constraint-cannot-cross-tables exists to say.
This replaced a regex matching only the search_count shape, which meant the
rule claimed ground its own checker never patrolled.
TWO NARROWINGS, BOTH FROM `nexa recall --aim`
---------------------------------------------
Placed against the method each fix edited, this rule scored 0 on-target and
47 other-symbol out of 49 -- the worst signal in the corpus by that measure.
Reading the 85 findings on OpenSPP gave two classes where the advice cannot
be followed at all, which is worse than advice nobody wanted:
The search is on ANOTHER model. `self.env["other.model"].search(...)`
tests uniqueness across tables, and a UNIQUE on this one cannot express
that. `endpoint_route_handler._check_route_unique_across_models` is the
clearest case: it loops over every consumer model in the registry. That
belongs to check-constraint-cannot-cross-tables, which already says so.
The class ALREADY declares the constraint being recommended. A model
carrying `models.Constraint("UNIQUE(code)", ...)` or an `_sql_constraints`
entry for the same column has taken the advice; the Python check beside it
is belt and braces, and telling the author to do the thing they did is how
a checker gets switched off. Both spellings are read, because Odoo 17
introduced the second and OpenSPP uses both.
"""
tree, _ = parse_python(code)
if tree is None:
return []
guarded = _columns_already_constrained(
tree, (facts or {}).get("odoo_series"))
findings = []
for fn in _functions(tree):
declared = _decorator_args(fn, "api", "constrains")
if declared is None or declared is UNREADABLE:
continue
roots = {d.split(".")[0] for d in declared}
if roots and roots <= guarded.get(id(_owner_of(tree, fn)), set()):
continue # the class already declares this constraint
searching = [n for n in ast.walk(fn)
if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute)
and n.func.attr in ("search", "search_count", "search_read")
and _searches_self(n)]
if searching:
findings.append((searching[0].lineno,
"{0}() tests uniqueness in Python -- two concurrent "
"transactions can both pass it and both insert; a UNIQUE "
"models.Constraint cannot race".format(fn.name)))
continue
if any(isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute)
and n.func.attr in ("search", "search_count", "search_read")
for n in ast.walk(fn)):
continue # searches another model: not this rule's
# Anything the database cannot see for itself disqualifies the whole
# function. Two exemptions, both necessary: calls inside the raise,
# because ValidationError(...) is how the finding gets reported rather
# than part of what is being decided, and the decorator list, because
# @api.constrains(...) is itself a Call and walking the whole function
# node would let every constraint disqualify itself.
in_raise = _message_only_attrs(fn)
body = [node for stmt in fn.body for node in ast.walk(stmt)]
if any(isinstance(n, ast.Call) and id(n) not in in_raise for n in body):
continue
reads = _field_reads(fn, _aliases(fn))
if not reads or any(len(path) != 1 or path[0] not in roots
for path, _ in reads):
continue
# Every condition has to BE a literal comparison, not merely contain
# one. `if record.storage == "url" and not record.url:` is a guard plus
# a presence check, and only the guard is an ast.Compare -- so the old
# test saw a column compared to a literal, concluded the function did
# nothing else, and said so. That shape is five of the eight hits this
# checker produced across two OCA repositories.
if any(not _is_literal_comparison(atom) for atom in _test_atoms(fn)):
continue
for node in ast.walk(fn):
if not isinstance(node, ast.Compare):
continue
operands = [node.left] + list(node.comparators)
if not any(isinstance(o, ast.Constant)
and isinstance(o.value, (int, float, str))
and not isinstance(o.value, bool) for o in operands):
continue
findings.append((node.lineno,
"{0}() compares its own declared column against a literal "
"and does nothing else -- the same rule as a CHECK in "
"models.Constraint holds for every writer, including "
"imports, XML-RPC and sudo".format(fn.name)))
break
return findings
def ondelete_leaves_a_dangling_read(code, facts=None):
"""odoo.ondelete-is-a-decision
Fires on a Many2one that declares no ondelete AND is read through somewhere
in the same module.
The ORM default for an OPTIONAL Many2one is 'set null'. Delete the parent and
the reference is nulled; read it afterwards and Odoo hands back an empty
recordset, whose attributes evaluate to False rather than raising. That is
the rule's whole complaint -- "dangling references that later reads treat as
empty" -- and it needs a read to happen at all.
Two exemptions, both because the default is already correct there:
required=True Odoo defaults a required Many2one to 'restrict', which is
what this rule recommends. 56 of the 198 hits on OCA were
fields already doing the right thing.
related= a related field mirrors another record's value and takes
its deletion behaviour from the source.
no column a compute= without store=True is never written, so there
is no foreign key for ondelete to govern.
TransientModel a wizard's record is vacuumed. The harm here is a nulled
reference read long afterwards, which needs a row that
lives long enough for the parent to be deleted under it.
NOT flagged, by choice: a Many2one declared and never dereferenced. Nulling a
reference nobody reads through changes no answer, and flagging it produced
4.1 findings per 1000 lines of reviewed OCA code -- enough to bury the other
nine Odoo rules under one that is a policy rather than a defect.
That makes the checker narrower than the rule, which still asks for an
explicit ondelete on every Many2one. The gap is deliberate and is the same
one required=True already had.
"""
tree, _ = parse_python(code)
if tree is None:
return []
declared = _many2one_without_ondelete(tree)
if not declared:
return []
findings = []
for name, (lineno, comodel) in sorted(declared.items(),
key=lambda kv: kv[1][0]):
reads = _dereferences_of(tree, name)
if not reads:
continue
trigger = context.parent_is_deleted(facts, comodel)
if trigger == context.ABSENT:
# The tree defines this parent and never deletes it. No delete, no
# nulled reference, no dangling read -- the rule's entire complaint
# is conditional on a thing that does not happen here.
continue
message = ("{0} declares no ondelete, so the ORM defaults it to "
"'set null' -- and {0}.{1} on line {2} reads through it, "
"which yields False rather than raising once the parent "
"is deleted".format(name, reads[0][0], reads[0][1]))
if trigger == context.PRESENT:
message += (", and this tree does delete {0}".format(comodel))
findings.append((lineno, message,
"defect" if trigger == context.PRESENT else None))
return findings
def _many2one_without_ondelete(tree):
"""{field name: (line, comodel)} for every fields.Many2one(...) that leaves
deletion behaviour to the ORM. required=True and related= are exempt.
The comodel travels with the declaration because the trigger is a question
about the PARENT -- whether anything deletes it -- and by the time the
finding is written the call node is gone."""
found = {}
transient = _transient_spans(tree)
for node in ast.walk(tree):
if not isinstance(node, ast.Assign) or len(node.targets) != 1:
continue
target = node.targets[0]
if not isinstance(target, ast.Name):
continue
call = node.value
if not (isinstance(call, ast.Call) and isinstance(call.func, ast.Attribute)
and call.func.attr == "Many2one"):
continue
if _within(transient, node.lineno) or _has_no_stored_column(call):
continue
exempt = False
for keyword in call.keywords:
if keyword.arg in ("ondelete", "related"):
exempt = True
elif keyword.arg == "required" and _is_true(keyword.value):
exempt = True
if not exempt:
found[target.id] = (node.lineno, _comodel_of(call))
return found
def _is_true(node):
return isinstance(node, ast.Constant) and node.value is True
def _is_sql_view(cls):
"""True for a model declaring `_auto = False`: a SQL view, not a table.
Odoo builds no table for one and therefore no foreign key, so `ondelete`
is not a decision anybody can make about it -- there is no constraint for
a value to be chosen for. 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.
"""
for node in cls.body:
if not isinstance(node, ast.Assign):
continue
if any(isinstance(t, ast.Name) and t.id == "_auto" for t in node.targets):
return isinstance(node.value, ast.Constant) and node.value.value is False
return False
def _transient_spans(tree):
"""Line ranges of every class this rule family does not apply to.
A wizard's record is vacuumed, so the failure both ondelete-is-a-decision
and stored-compute-needs-complete-depends describe -- a value written once
and read wrong much later -- has no time in which to happen. A `_auto =
False` model has no table at all, which rules out the same failure for a
different reason: see _is_sql_view. Returned as spans because the
declarations and methods they gate are found by walking the whole tree,
which loses the class on the way down.
check_company_missing_on_company_owned_relation already had this exemption
and iterates classes directly; these two do not, hence the spans.
"""
return [(n.lineno, getattr(n, "end_lineno", n.lineno))
for n in ast.walk(tree)
if isinstance(n, ast.ClassDef) and (_is_transient(n) or _is_sql_view(n))]
def _within(spans, lineno):
return any(start <= lineno <= end for start, end in spans)
def _has_no_stored_column(call):
"""Does this field declaration put nothing in the database?
compute= without store=True is calculated on read and never written, so
there is no column for ondelete to act on and nothing for check_company to
constrain. Both rules are about what the database HOLDS, and advice about a
column that does not exist cannot be taken.
"""
computed = stored = False
for keyword in call.keywords:
if keyword.arg == "compute":
computed = True
elif keyword.arg == "store":
stored = _is_true(keyword.value)
return computed and not stored
def _dereferences_of(tree, field):
"""[(attribute, line)] for every UNGUARDED <x>.<field>.<attribute> in the tree.
The outer attribute is what makes it a read THROUGH the relation rather than
a read OF it. `rec.partner_id` alone is a recordset test and is falsy-safe;
`rec.partner_id.name` is the one that quietly becomes False.
Reads inside a function that tests the field for truthiness are left out.
The rule's complaint is that a nulled reference "yields False rather than
raising", and an author who wrote `if not self.model_id: return` has already
answered it -- the reference may dangle and nothing reads through it when it
does. Measured on OpenSPP, that guard is the majority idiom, and flagging it
made the difference between a rule about a defect and a rule about a
construct.
Function-level, deliberately. The dominant shape is a guard at the top of a
method covering everything below it, and following the value any further
than that is dataflow analysis this does not have.
A read is placed by the field's name, not by the model it is read on.
`mapped("<field>.<attribute>")` is a read too and is left out: measured on
the three trees it added one finding, a quant's location read in
stock.location's file, and five of the thirteen such reads were another
model's field of the same name (ARCHITECTURE 3.79).
"""
guarded = set()
for fn in _functions(tree):
if _tests_for_emptiness(fn, field):
guarded.update(id(node) for node in ast.walk(fn))
reads = []
for node in ast.walk(tree):
if not isinstance(node, ast.Attribute) or id(node) in guarded:
continue
inner = node.value
if not (isinstance(inner, ast.Attribute) and inner.attr == field):
continue
if node.attr in ORM_ATTRS:
continue
reads.append((node.attr, node.lineno))
return sorted(reads, key=lambda r: r[1])
def _tests_for_emptiness(fn, field):
"""Does this function check whether <x>.<field> is set, anywhere in it?
`if not self.model_id: return`, `if rec.partner_id:`, `rec.partner_id and
rec.partner_id.name`, and the ternary spelling all count. What is looked for
is the field appearing as a CONDITION rather than as a value, which is the
difference between asking whether it is there and assuming it is.
"""
def names_the_field(node):
while isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not):
node = node.operand
return isinstance(node, ast.Attribute) and node.attr == field
for node in ast.walk(fn):
tests = []
if isinstance(node, (ast.If, ast.While, ast.IfExp)):
tests.append(node.test)
elif isinstance(node, ast.BoolOp):
tests.extend(node.values)
elif isinstance(node, ast.Assert):
tests.append(node.test)
elif isinstance(node, ast.comprehension):
tests.extend(node.ifs)
for test in tests:
if names_the_field(test):
return True
if isinstance(test, ast.BoolOp) and any(
names_the_field(value) for value in test.values):
return True
return False
# Core models that carry a company_id of their own, so a relation to one can
# point across a tenant boundary. Curated rather than derived: Nexa has no Odoo
# registry to ask, and a guessed list would fire on every Many2one in the corpus.
COMPANY_OWNED = frozenset([
"account.journal", "account.account", "account.payment.term",
"account.analytic.account", "stock.warehouse", "stock.location",
"stock.picking.type", "product.pricelist", "res.partner.bank",
])
# Models that ARE the company, or are scoped to one by construction. A
# check_company on a field of res.company has nothing to compare against; Odoo's
# own fields.py carries a special branch for exactly that case.
NOT_COMPANY_SCOPED = frozenset(["res.company", "res.users"])
def check_company_missing_on_company_owned_relation(code, facts=None):
"""odoo.check-company-on-relations
Fires on a Many2one or Many2many to a company-owned model, declared without
check_company, in a model that is itself company-scoped.
All three conditions matter, and the regex that preceded this could express
only the first two. Firing without the third is what produced 17 of its 37
hits on OCA:
six were on res.company itself, where check_company has nothing to compare
against -- odoo/fields.py has a branch for that case;
eleven were TransientModel wizards. A wizard's records do not persist, and
the harm this rule names is a multi-company client seeing another tenant's
data on their DOCUMENT. Flagging wizards is not wrong so much as it is a
different, much smaller problem wearing a critical rule's severity.
"Company-scoped" is read from the code, not guessed: the class declares a
company_id field of its own. A class that only inherits one cannot be
resolved without an Odoo registry, so it is skipped rather than assumed --
silence over a false critical.
A module-level declaration with no class around it is treated as one
implicit model body. That is the shape a generated answer arrives in, and
real modules always put fields in a class, so it costs nothing on real code.
"""
trigger = context.multi_company_in_use(facts)
if trigger == context.ABSENT:
# One company cannot leak to another. The rule is about a boundary this
# codebase does not have, and every finding it could make here would be
# about a harm with no mechanism.
return []
tree, _ = parse_python(code)
if tree is None:
return []
classes = [n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)]
scopes = classes if classes else [tree]
findings = []
for scope in scopes:
if isinstance(scope, ast.ClassDef):
if _model_names(scope) & NOT_COMPANY_SCOPED:
continue
if _is_transient(scope) or _is_sql_view(scope):
# A SQL view has no table, so no foreign key for check_company
# to add a guard to: the rows come from the query. OpenSPP's
# fund_report was the one live false positive left for this rule.
continue
if not _declares_company_id(scope):
continue
for node in ast.walk(scope):
if not (isinstance(node, ast.Call)
and isinstance(node.func, ast.Attribute)
and node.func.attr in ("Many2one", "Many2many")):
continue
comodel = _comodel_of(node)
if comodel not in COMPANY_OWNED:
continue
if any(k.arg == "check_company" for k in node.keywords):
continue
if any(k.arg == "related" for k in node.keywords):
# A mirror of a field on another record. The constraint belongs
# on the source, and the source is flagged in its own right --
# so this was also reporting the same decision twice.
continue
if _has_no_stored_column(node):
# No column, nothing to constrain.
continue
message = ("relation to {0} with no check_company, on a model "
"that has its own company_id -- a record in one "
"company can reference another company's {0}"
.format(comodel))
if trigger == context.PRESENT:
message += ", and this codebase runs multi-company"
findings.append((node.lineno, message,
"defect" if trigger == context.PRESENT else None))
return findings
def _model_names(cls):
"""Every model name the class claims: _name and _inherit, string forms."""
names = set()
for node in cls.body:
if not isinstance(node, ast.Assign) or len(node.targets) != 1:
continue
target = node.targets[0]
if not (isinstance(target, ast.Name) and target.id in ("_name", "_inherit")):
continue
value = node.value
if isinstance(value, ast.Constant) and isinstance(value.value, str):
names.add(value.value)
elif isinstance(value, (ast.List, ast.Tuple)):
for item in value.elts:
if isinstance(item, ast.Constant) and isinstance(item.value, str):
names.add(item.value)
return names
def _is_transient(cls):
for base in cls.bases:
if isinstance(base, ast.Attribute) and base.attr == "TransientModel":
return True
if isinstance(base, ast.Name) and base.id == "TransientModel":
return True
return False
def _declares_company_id(scope):
body = scope.body if isinstance(scope, (ast.ClassDef, ast.Module)) else []
for node in body:
if not isinstance(node, ast.Assign) or len(node.targets) != 1:
continue
target = node.targets[0]
if isinstance(target, ast.Name) and target.id == "company_id":
return True
return False
def _comodel_of(call):
"""The model a relational field points at, positional or keyword."""
if call.args and isinstance(call.args[0], ast.Constant) \
and isinstance(call.args[0].value, str):
return call.args[0].value
for keyword in call.keywords:
if keyword.arg == "comodel_name" and isinstance(keyword.value, ast.Constant):
return keyword.value.value
return None
# ---------------------------------------------------- the hierarchy checker
# Either spelling clears the rule. _check_recursion still works on 18.0 -- it
# warns and delegates to _has_cycle -- so code carrying it is out of date, not
# unguarded.
CYCLE_GUARDS = frozenset(["_has_cycle", "_check_recursion"])
def parent_can_form_a_cycle(code):
"""odoo.recursion-check-on-hierarchy
Fires on a `parent_id` Many2one that points at its own model, in a model
nothing protects from a cycle.
Was a regex for `parent_id = fields.Many2one`, cleared by either guard's
name anywhere in the file. It could not ask the two questions the harm
depends on, and both produced false positives that were classified true:
Does the field point at its own model? A `parent_id` to a different
model is an ordinary foreign key and cannot close a loop. OCA's
account.cash.deposit.line.parent_id points at account.cash.deposit.
Does the framework already refuse the cycle? A model with
`_parent_store = True` keeps parent_path, and _parent_store_update
raises "Recursion Detected." on any write that would make a record its
own ancestor (odoo/models.py, present since 2019). Six of the seven
verdicts once marked true -- account.asset.group on OCA, and spp.area,
spp.area.type, spp.audit.rule, spp.consent.purpose and
spp.hazard.category on OpenSPP -- were models doing exactly this.
The one that survived both questions is the shape the rule is for:
spp.dms.directory declares a parent_path CHAR but never sets
_parent_store, so nothing maintains it and nothing refuses a loop, while
its complete_name and root_directory_id computes recurse up the chain.
Self-reference is read through the three spellings real modules use: a
string, the class's own `_name` variable, and a module constant both are
assigned from (`_name = _type_model`). A comodel that cannot be resolved
keeps the old reading and fires, and so does a module-level declaration
with no class around it -- the shape a generated answer arrives in.
"""
tree, _ = parse_python(code)
if tree is None:
return []
for node in ast.walk(tree):
name = node.attr if isinstance(node, ast.Attribute) else \
node.id if isinstance(node, ast.Name) else None
if name in CYCLE_GUARDS:
return []
constants = _module_string_constants(tree)
classes = [n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)]
findings = []
for scope in classes or [tree]:
own = None
if isinstance(scope, ast.ClassDef):
if _parent_store_guards_parent_id(scope) or _is_sql_view(scope):
# A view is written by nobody: its parent comes from the query
# over the tables underneath, which is where a guard belongs.
continue
own = _own_model_name(scope, constants)
for node in scope.body:
if not (isinstance(node, ast.Assign) and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)
and node.targets[0].id == "parent_id"
and isinstance(node.value, ast.Call)
and isinstance(node.value.func, ast.Attribute)
and node.value.func.attr == "Many2one"):
continue
if not node.value.args and not any(k.arg == "comodel_name"
for k in node.value.keywords):
# `parent_id = fields.Many2one(tracking=3)` names no model: it
# re-declares attributes of a field declared elsewhere, and the
# relation, with any guard on it, lives with that declaration.
continue
comodel = _resolved_comodel(node.value, constants, own)
if own is not None and comodel is not None and comodel != own:
continue
findings.append((node.lineno,
"parent_id points at {0} itself and nothing refuses a "
"cycle -- a record made its own ancestor loops every "
"recursive read until the worker is killed. "
"_parent_store = True would refuse it, and so would an "
"@api.constrains calling _has_cycle()".format(
comodel or own or "its own model")))
return findings
def _module_string_constants(tree):
"""NAME = "string" at module level: how a model name gets shared."""
found = {}
for node in tree.body:
if (isinstance(node, ast.Assign) and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)
and isinstance(node.value, ast.Constant)
and isinstance(node.value.value, str)):
found[node.targets[0].id] = node.value.value
return found
def _class_assignment(cls, attr):
"""The value a class-body assignment gives `attr`, chained ones included.
Odoo's own test models write `_name = _description = 'x'`; reading only
single-target assignments left those classes nameless, so a parent_id to
a different model read as unresolvable and fired.
"""
for node in cls.body:
if isinstance(node, ast.Assign) and any(
isinstance(t, ast.Name) and t.id == attr for t in node.targets):
return node.value
return None
def _own_model_name(cls, constants):
"""The model this class defines or extends, when one name says it."""
value = _class_assignment(cls, "_name") or _class_assignment(cls, "_inherit")
if isinstance(value, ast.Constant) and isinstance(value.value, str):
return value.value
if isinstance(value, ast.Name):
return constants.get(value.id)
return None
def _parent_store_guards_parent_id(cls):
"""True when the ORM itself refuses a cycle through parent_id."""
store = _class_assignment(cls, "_parent_store")
if not _is_true(store):
return False
# _parent_store maintains the field _parent_name names, parent_id unless
# said otherwise. A model storing some other chain leaves parent_id bare.
name = _class_assignment(cls, "_parent_name")
return name is None or (isinstance(name, ast.Constant) and name.value == "parent_id")
def _resolved_comodel(call, constants, own):
literal = _comodel_of(call)
if literal is not None:
return literal
expressions = [call.args[0]] if call.args else []
expressions += [k.value for k in call.keywords if k.arg == "comodel_name"]
for expr in expressions:
if isinstance(expr, ast.Name):
if expr.id == "_name":
return own
if expr.id in constants:
return constants[expr.id]
return None
# ------------------------------------------------------ the XML data checker
# Models whose records an administrator tunes through the UI after install, so
# that an upgrade rewriting them undoes a decision somebody made on purpose.
#
# Chosen by measurement, not by opinion. Odoo core puts its own records of each
# under noupdate almost without exception -- ir.config_parameter 26 of 28,
# ir.sequence 16 of 17, decimal.precision 7 of 8, and stages 30 of 34 across
# seven stage models -- while views, actions and reports, which must upgrade,
# sit at 0 to 2%. ir.cron is the exception to that method: core protects only
# 39 of 93. It came in on an incident: OpenSPP's #473 is two crons and two
# parameters in one file with no noupdate, and names the harm in its title --
# an admin who changed an interval or disabled a cron is reset by the next
# upgrade. #479 fixed it with noupdate and a migration. Reading every cron
# finding across the three trees then narrowed it to one shape: see
# editable_record_without_noupdate.
#
# ir.rule is out on purpose, although core protects 566 of 598. A record rule
# frozen by noupdate never receives the security fix its module ships later,
# which is a trade-off a checker has no business deciding for anybody.
EDITABLE_DATA_MODELS = frozenset([
"ir.config_parameter", "ir.sequence", "decimal.precision", "ir.cron",
])
def _is_editable_data_model(model):
return model in EDITABLE_DATA_MODELS or (model or "").endswith(".stage")
def _is_false_literal(text):
return text.strip().lower() in ("false", "0")
def parse_xml(code):
"""(True, None), or (False, (line, message)) when expat cannot read it."""
parser = xml.parsers.expat.ParserCreate()
try:
parser.Parse(code.encode("utf-8"), True)
except xml.parsers.expat.ExpatError as exc:
return False, (exc.lineno, xml.parsers.expat.errors.messages.get(
exc.code, "not well-formed"))
return True, None
# An Odoo data document. An answer from the model can carry one inside prose
# and Python, where the whole text is not XML; the documents inside it still are.
_XML_DOCUMENT = re.compile(r"<(odoo|openerp)\b.*?</\1\s*>", re.S)
def _xml_documents(code):
"""(line offset, text) for each Odoo data document the code holds."""
ok, _ = parse_xml(code)
if ok:
return [(0, code)]
return [(code.count("\n", 0, m.start()), m.group(0))
for m in _XML_DOCUMENT.finditer(code)]
def _records_outside_noupdate(document):
"""(line, model, xml id, ships disabled) for each <record> no noupdate="1"
reaches.
noupdate is inherited from the enclosing <odoo>, <openerp> or <data>, and
an inner element can switch it back off, so it is tracked per element.
`ships disabled` is whether the record sets its `active` field false.
"""
stack, found = [], []
current = {}
parser = xml.parsers.expat.ParserCreate()
def start(tag, attrs):
inherited = stack[-1] if stack else False
if attrs.get("noupdate") is not None:
inherited = attrs["noupdate"].strip().lower() in ("1", "true")
stack.append(inherited)
if tag == "record" and not inherited:
current.clear()
current.update(line=parser.CurrentLineNumber, model=attrs.get("model"),
id=attrs.get("id"), disabled=False, text=None)
elif tag == "field" and current and attrs.get("name") == "active":
if attrs.get("eval") is not None:
current["disabled"] = _is_false_literal(attrs["eval"])
else:
current["text"] = []
def chars(data):
if current.get("text") is not None:
current["text"].append(data)
def end(tag):
stack.pop()
if tag == "field" and current.get("text") is not None:
current["disabled"] = _is_false_literal("".join(current["text"]))
current["text"] = None
elif tag == "record" and current:
found.append((current["line"], current["model"], current["id"],
current["disabled"]))
current.clear()
parser.StartElementHandler, parser.EndElementHandler = start, end
parser.CharacterDataHandler = chars
try:
parser.Parse(document.encode("utf-8"), True)
except xml.parsers.expat.ExpatError:
return []
return found
def editable_record_without_noupdate(code):
"""odoo.noupdate-on-user-editable-data
Fires on each <record> of a model administrators tune (see
EDITABLE_DATA_MODELS) that no noupdate="1" reaches, at the record's own
line.
Was a regex for `<odoo>` immediately followed by `<record`, cleared by the
word noupdate anywhere in the file. So it fired once, on <odoo>, for any
record at all -- a view, a menu, an action, all of which have to upgrade --
and one noupdate block anywhere silenced every record outside it. OpenSPP's
spp_starter_sp_mis config_parameters.xml after its own fix still carries a
parameter outside the block, and the regex called the file clean.
It had also never been fired at real code: fieldtest and recall read .py
files only, and this rule reads XML. Its `unproven` grade was a statement
about where the tools looked.
"""
findings = []
for offset, document in _xml_documents(code):
for line, model, xid, disabled in _records_outside_noupdate(document):
if not _is_editable_data_model(model):
continue
if model == "ir.cron" and not disabled:
# A cron's schedule is usually the mechanism, not a setting:
# 57 of 62 cron findings across the three trees read false, and
# core keeps 54 of its own 93 crons updatable on purpose. The
# shape that held up ships disabled for an operator to switch
# on -- which is exactly the decision an upgrade then undoes.
continue
findings.append((offset + line,
"{0} record '{1}' has no noupdate=\"1\" -- every module "
"upgrade rewrites it with the shipped value, silently "
"undoing what an administrator changed. Adding noupdate "
"now does not reach databases that already have it: "
"ir_model_data.noupdate also needs flipping in a "
"migration".format(model, xid or "?")))
return findings
# ------------------------------------------------------- the cursor checkers
# The names an Odoo database cursor is reached by. self.env.cr and self._cr are
# the two the framework provides; `cr` alone is the argument name every
# migration script and post-init hook uses. Anything else called .execute() or
# .commit() is somebody else's object and none of our business.
CURSOR_NAMES = frozenset(["cr", "_cr", "cursor", "_cursor"])
def _cursor_call(node, method):
"""True when this Call is <something>.cr.<method>(...).
The attribute immediately before the method name is what decides it, so
self.env.cr.execute, self._cr.execute and a bare cr.execute all match while
connection.execute and session.commit do not.
"""
if not isinstance(node, ast.Call):
return False
func = node.func
if not isinstance(func, ast.Attribute) or func.attr != method:
return False
owner = func.value
if isinstance(owner, ast.Attribute):
return owner.attr in CURSOR_NAMES
if isinstance(owner, ast.Name):
return owner.id in CURSOR_NAMES
return False
def _root_name(node):
"""The name a dotted chain starts from. self.env.cr -> 'self'."""
while isinstance(node, ast.Attribute):
node = node.value
return node.id if isinstance(node, ast.Name) else None
def _independent_cursors(scope):
"""Names in this scope standing for a cursor the caller does not own.
`with Registry(...).cursor() as new_cr` opens a SECOND connection, and an
Environment built on it commits its own transaction. That is not the
caller's unit of work being split -- it is the documented way to write a
batch job that must not hold one transaction open for the whole run, and
the rule's own instruction is "let the caller own the transaction", which
this already does.
Bound POSITIVELY, by watching the cursor be opened, rather than by treating
anything not spelled self.env.cr as independent. The two field instances of
this rule are exactly the two sides of that line: OpenSPP's expiry cron
commits self.env.cr inside its loop and is a true positive its author marked
and kept, while OCA's batch_unlink commits a cursor it opened three lines
earlier and is not.
"""
names = set()
for _ in range(3):
before = len(names)
for node in ast.walk(scope):
if isinstance(node, ast.withitem):
call = node.context_expr
if (isinstance(call, ast.Call)
and isinstance(call.func, ast.Attribute)
and call.func.attr == "cursor"
and isinstance(node.optional_vars, ast.Name)):
names.add(node.optional_vars.id)
elif (isinstance(node, ast.Assign) and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)
and isinstance(node.value, ast.Call)):
# api.Environment(new_cr, ...) carries the new cursor forward.
if any(isinstance(a, ast.Name) and a.id in names
for a in node.value.args):
names.add(node.targets[0].id)
if len(names) == before:
break
return names
def commit_inside_a_loop(code):
"""odoo.commit-inside-a-loop-leaves-half-written-data
A commit ends the transaction the ORM is working inside. Done once per
iteration, the loop stops being one unit of work: iteration seven raising
leaves iterations one to six already written, the caller's except clause
has nothing left to roll back, and the user sees an error over data that
stayed.
Only loops, not every commit. A commit in a cron batch or a migration
script is a deliberate decision about batch size, and a rule that fired on
all of them would be argued with rather than read. Inside a loop over a
recordset it is almost never deliberate -- it is somebody flushing as they
go -- and that is the shape this reports.
The receiver decides, not the method name: `.commit()` on anything that is
not a cursor belongs to some other library.
"""
tree, _ = parse_python(code)
if tree is None:
return []
# Which names mean an independent cursor, per function. Scoped rather than
# gathered over the file, so a `new_cr` opened in one method cannot excuse a
# commit in another that happens to reuse the name.
spans = [(fn.lineno, getattr(fn, "end_lineno", fn.lineno),
_independent_cursors(fn)) for fn in _functions(tree)]
def independent_at(lineno):
best = None
for start, end, names in spans:
if start <= lineno <= end and (best is None or start > best[0]):
best = (start, names)
return best[1] if best else set()
findings = []
for loop in ast.walk(tree):
if not isinstance(loop, (ast.For, ast.AsyncFor, ast.While)):
continue
# The loop's own body, not its else clause: an else runs once, after
# the loop, and a commit there is a commit after a loop.
for statement in loop.body:
for node in ast.walk(statement):
if not _cursor_call(node, "commit"):
continue
if _root_name(node.func.value) in independent_at(node.lineno):
continue
findings.append((
node.lineno,
"cr.commit() inside a loop -- an exception on a later "
"iteration leaves the earlier ones written, and the "
"caller cannot roll the batch back"))
return sorted(set(findings))
def sql_built_by_string_formatting(code):
"""odoo.sql-needs-query-parameters
cr.execute("... id = %s", (rec.id,)) is correct and contains a percent
sign. cr.execute("... id = %s" % rec.id) is the bug and contains the same
percent sign. A regex cannot tell them apart, which is why this is an AST
pass: the difference is whether the substitution happens in Python before
the cursor sees the string, and that is a BinOp rather than a character.
Four spellings, all of them the same mistake:
"..." % value BinOp with Mod
"...".format(value) a call to .format on a string
f"...{value}..." JoinedStr
"..." + value BinOp with Add, one side a string
Only the FIRST argument is examined. The second is the parameter tuple and
building that with formatting is somebody's business, not a SQL injection.
"""
tree, _ = parse_python(code)
if tree is None:
return []
findings = []
for node in ast.walk(tree):
if not _cursor_call(node, "execute") or not node.args:
continue
how = _how_the_query_was_built(node.args[0])
if how:
findings.append((
node.args[0].lineno,
"the query handed to cr.execute() is built by {0} -- pass the "
"values as the second argument so the driver quotes them".format(how)))
return sorted(set(findings))
def _how_the_query_was_built(node):
"""The name of the string operation that produced this query, or ""."""
if isinstance(node, ast.JoinedStr):
return "an f-string"
if isinstance(node, ast.BinOp):
if isinstance(node.op, ast.Mod):
return "percent formatting"
if isinstance(node.op, ast.Add) and (_is_a_string(node.left)
or _is_a_string(node.right)):
return "string concatenation"
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and node.func.attr == "format" and not _is_composed_sql(node.func.value):
return "str.format()"
return ""
def _is_composed_sql(node):
"""psycopg2's sql.SQL("... {}").format(sql.Identifier(name)) is the CORRECT
way to parameterise a table or column name, and it spells .format().
Found by running this over 229,600 lines of OpenSPP: the one finding that
was wrong rather than merely suppressed was this construct, on a DROP INDEX
whose identifier the surrounding code had already validated. A checker that
tells an author to stop using the safe API is worse than one that says
nothing.
"""
if not isinstance(node, ast.Call):
return False
func = node.func
if isinstance(func, ast.Attribute):
return func.attr in ("SQL", "Identifier", "Composed", "Literal")
return isinstance(func, ast.Name) and func.id in ("SQL", "Identifier")
def _is_a_string(node):
return ((isinstance(node, ast.Constant) and isinstance(node.value, str))
or isinstance(node, ast.JoinedStr))
# Odoo's exception hierarchy, read out of odoo/exceptions.py and identical in
# 16.0 through 18.0. Every one of these subclasses UserError, which is why
# catching UserError first makes the specific handler below it dead code.
ODOO_EXCEPTION_PARENT = {
"AccessDenied": "UserError",
"AccessError": "UserError",
"MissingError": "UserError",
"ValidationError": "UserError",
"UserError": "Exception",
"RedirectWarning": "Exception",
}
def _is_subclass_of(inner, outer):
"""Would `except outer` catch something raised as `inner`?
Odoo's hierarchy comes from the table above; everything else is asked of
the interpreter, so builtins stay correct without a second list to
maintain. An unknown name answers False -- a class this pass has never
heard of is not evidence of anything.
"""
if inner == outer:
return True
seen = set()
cursor = inner
while cursor in ODOO_EXCEPTION_PARENT and cursor not in seen:
seen.add(cursor)
cursor = ODOO_EXCEPTION_PARENT[cursor]
if cursor == outer:
return True
if outer in ("BaseException", "Exception") and (
inner in ODOO_EXCEPTION_PARENT or _builtin_exception(inner)):
return outer == "BaseException" or inner != "BaseException"
first, second = _builtin_exception(inner), _builtin_exception(outer)
return bool(first and second and issubclass(first, second))
def _builtin_exception(name):
found = getattr(builtins, name, None)
return found if isinstance(found, type) and issubclass(found, BaseException) else None
def _handler_names(handler):
"""The exception names one `except` clause catches, tuples included."""
node = handler.type
if node is None:
return [] # a bare except, which Python forces last
parts = node.elts if isinstance(node, ast.Tuple) else [node]
names = []
for part in parts:
if isinstance(part, ast.Name):
names.append(part.id)
elif isinstance(part, ast.Attribute):
names.append(part.attr) # odoo.exceptions.AccessError
return names
def handler_order_shadows_a_subclass(code):
"""odoo.except-order-follows-the-hierarchy
Python tries handlers in order and takes the first that matches, so a base
class placed above a subclass makes the lower handler dead code. Nothing
warns: it parses, it imports, it runs, and the specific branch is simply
never entered. AccessDenied, AccessError, MissingError and ValidationError
all subclass UserError -- read out of odoo/exceptions.py in 16.0 and 18.0,
where the hierarchy is identical. Builtins are asked of the interpreter
rather than listed, so `except Exception` above `except KeyError` is the
same finding for the same reason.
HOW THIS RULE CAME ABOUT, INCLUDING THE PART THAT WAS WRONG
-----------------------------------------------------------
A tally over the 62 OpenSPP fixes the corpus was silent on said NINE of them
added the name `AccessError` or `MissingError`, which read as the largest
missing-rule cluster in that list. This checker was written from that count.
It catches none of those nine. Reading the actual diffs afterwards -- which
the count was standing in for, and should not have been -- the before-state
is one handler, not two in the wrong order:
try:
service.submit(cr)
except UserError as e:
raise HTTPException(status_code=409, ...)
AccessError is not mishandled there, it is unhandled: swallowed by the base
class and reported as 409 Conflict. Nothing shadows anything, and "caught
the base class without distinguishing a subclass" is not statically
decidable as a defect, because `except UserError` is correct in most code.
So the tally was a bad proxy for the diffs, which is the same mistake
SCOPE_TOKENS makes when it counts a commit in scope for mentioning
ValidationError.
The construct this DOES catch is a real defect and the rule is kept for it,
but with nothing behind it: zero findings on 229,600 lines of OpenSPP and no
incident in any history measured here. It ships `advisory` on the second
ground -- unproven rather than a preference -- which is exactly what that
label is for.
"""
tree, _ = parse_python(code)
if tree is None:
return []
findings = []
for node in ast.walk(tree):
if not isinstance(node, ast.Try):
continue
earlier = []
for handler in node.handlers:
names = _handler_names(handler)
for outer, outer_line in earlier:
for inner in names:
if inner != outer and _is_subclass_of(inner, outer):
findings.append((
handler.lineno,
"'except {0}' on line {1} already catches {2}, so "
"this handler never runs -- {2} subclasses {0}".format(
outer, outer_line, inner)))
earlier.extend((name, handler.lineno) for name in names)
return sorted(set(findings))
def depends_names_unread_fields(code):
"""odoo.depends-must-not-name-unread-fields
The other direction of depends_covers_compute_reads, and a milder problem:
a declared dependency the body never reads does not make the value stale,
it makes the value recompute on writes that cannot change it.
Harvested from OpenSPP's history rather than reasoned about. Two fix
commits, both of them a leftover:
@api.depends("partner_id", "pathway_id", "assessment_date")
def _compute_name(self): # reads partner_id and pathway_id only
@api.depends("topic_ids")
def _compute_session_count(self): # counts rows on another model
The second is why this is worth reporting at all. The decorator is the only
statement anywhere of what a stored value depends on, and a reader trusts
it. Left naming a field an older body used to read, it hides that this
compute has no expressible dependency at all -- it queries a different
model -- and the staleness that follows looks accounted for.
Deliberately permissive, because a wrong entry costs a recompute and a
false finding costs trust:
A string literal anywhere in the body counts as a read. filtered("x"),
mapped("a.b") and sorted(key=...) name fields without an attribute access
ever appearing.
A declared path is covered by any read that shares a prefix with it in
either direction, so declaring line_ids while reading line_ids.price is
fine and so is the reverse.
A body that calls a method on self, or hands a record to any call, is
left alone entirely. The read is then inside the callee, this pass cannot
follow it, and the honest answer is not to guess. Measured on OpenSPP,
this is the largest false-positive class: a compute that declares five
fields and passes the record to a resolver that reads all five.
A body that reads a field the decorator does not declare is left alone
too. The two are already out of step, and that disagreement belongs to
depends_covers_compute_reads -- reporting it twice from opposite
directions is two findings about one confusion. It is also how a compute
reading another stored compute looks from here, and those declare the
upstream source on purpose. "Reads" here are the ones that rule makes,
fields named in a string (_named_reads) among them, so the two agree
about what the body reads.
"""
tree, _ = parse_python(code)
if tree is None:
return []
findings = []
for fn in _functions(tree):
declared = _decorator_args(fn, "api", "depends")
if not declared or declared is UNREADABLE:
continue
if _calls_a_method_on_self(fn) or _hands_a_record_to_a_call(fn) \
or _queries_another_model(fn) or _calls_super(fn):
# Not exempt: a body that only assigns constants. On core four of
# them were a stub for overrides or a deliberate reset, and on
# OpenSPP and OCA three were leftovers classified true by hand --
# "Legacy method", a target model that is always res.partner. The
# file cannot tell intent, so that shape stays a finding.
continue
aliases = _aliases(fn)
read = set(".".join(path) for path, _ in
_field_reads(fn, aliases) + _named_reads(fn, aliases))
produced = set(_assigned_attrs(fn))
if _reads_outside_the_declarations(read - produced, declared):
continue
read |= produced
read |= _string_literals(fn)
unread = [entry for entry in declared
if not _covered_by_a_read(entry, read)]
if unread:
# One finding per method, not per entry. Three leftovers on one
# decorator is one thing to fix, and three lines all pointing at
# the same def read as three problems.
findings.append((
fn.lineno,
"{0}() declares {1} in @api.depends and never reads {2} -- the "
"value recomputes on writes that cannot change it, and the "
"decorator stops describing the method".format(
fn.name, ", ".join(repr(str(e)) for e in unread),
"them" if len(unread) > 1 else "it")))
return sorted(set(findings))
def _calls_super(fn):
"""Does the body call super()?
Odoo merges @api.depends across the MRO: Field.get_depends walks every
override of the compute and extends one list with each one's _depends
(odoo/fields.py, 18.0 at 3c3e3b3). So an override's decorator ADDS
triggers for the chain, and the fields it names are read by the parent's
body, reached through super(). Core writes it as a comment: "EXTENDS 'web'
to add dependencies". Its own body reading nothing is the point. Three of
the fifteen sampled on odoo core were this, and all 40 findings the
exemption removed from core were an override that delegates to super().
"""
for node in _walk_body(fn):
if (isinstance(node, ast.Call) and isinstance(node.func, ast.Name)
and node.func.id == "super"):
return True
return False
def _reads_outside_the_declarations(read, declared):
"""Does the body read a field no declared entry covers?
Prefix-wise in both directions, so reading partner_id.name against a
declared partner_id is inside the declarations and reading
intervention_count against a declared intervention_ids is outside them.
"""
for path in read:
if not any(path == entry or path.startswith(entry + ".")
or entry.startswith(path + ".") for entry in declared):
return True
return False
def _hands_a_record_to_a_call(fn):
"""Does any call in the body receive a record as an argument?
resolve(scope) can read every field on scope, so a dependency declared for
its benefit is correct and invisible here. A domain built from rec.id is
not this: an id is an integer, and no callee reads a field through one.
"""
aliases = _aliases(fn)
for node in _walk_body(fn):
if not isinstance(node, ast.Call):
continue
for argument in list(node.args) + [kw.value for kw in node.keywords]:
if _is_record_valued(argument, aliases):
return True
return False
QUERY_METHODS = frozenset(["search", "search_count", "search_read", "read_group",
"_read_group", "execute", "_search"])
def _queries_another_model(fn):
"""Does the body count or fetch rows from somewhere other than self?
The largest class of false finding here by a wide margin: 17 of the 31 this
reported on OpenSPP. A compute whose value comes from rows in ANOTHER model
has no dependency @api.depends can express -- the ORM cannot watch a table
this record has no field for -- so whatever the author declared is their
best available approximation, usually the inverse One2many, and it often
works. Telling them to delete it removes the only recompute trigger the
value has.
The cost of the exclusion is real and stated rather than hidden: one of the
two harvested incidents this checker was written from is exactly this shape
-- @api.depends("topic_ids") on a compute that counted rows on another
model -- and is no longer reported. Reading it, the author's fix was to
decide there was no correct dependency at all, which is a judgement this
pass has no way to make for them.
"""
for node in _walk_body(fn):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and node.func.attr in QUERY_METHODS:
return True
return False
def _is_record_valued(node, aliases):
"""Is this argument a record, or a field of one, rather than a plain value?
Collection literals are looked inside, because a domain is a list of tuples
and a vals dict is a dict of them. Attribute chains are NOT walked into:
the Name at the base of rec.id is not itself an argument, and counting it
made a search domain look like a record being handed over.
"""
if isinstance(node, ast.Name):
return node.id in aliases
if isinstance(node, ast.Attribute):
path = _attr_path(node, aliases)
return bool(path) and path[0] not in ORM_ATTRS \
and not path[0].startswith("_")
if isinstance(node, (ast.List, ast.Tuple, ast.Set)):
return any(_is_record_valued(item, aliases) for item in node.elts)
if isinstance(node, ast.Dict):
return any(_is_record_valued(item, aliases)
for item in list(node.keys) + list(node.values) if item)
return False
def _covered_by_a_read(entry, read):
"""A declared path is covered by any read sharing a prefix with it."""
for path in read:
if path == entry or path.startswith(entry + ".") \
or entry.startswith(path + "."):
return True
return False
def _walk_body(fn):
"""Every node in the function's BODY, decorators excluded.
ast.walk(fn) includes the decorator list, and @api.depends("x") holds the
string "x". Counted as evidence the body mentions x, every declared entry
proves itself and the checker silently finds nothing -- which is exactly
what it did on the incident it was written from.
"""
for statement in fn.body:
for node in ast.walk(statement):
yield node
def _string_literals(fn):
"""Every string in the body, plus each dotted path inside one.
filtered("is_late") and mapped("line_ids.price") name fields without an
attribute access ever appearing, and a declared dependency on either is
correct. Splitting the dotted ones lets a declared prefix match too.
"""
out = set()
for node in _walk_body(fn):
if isinstance(node, ast.Constant) and isinstance(node.value, str):
text = node.value.strip()
if text:
out.add(text)
out.update(re.findall(r"[A-Za-z_][A-Za-z0-9_.]*", text))
return out
def _calls_a_method_on_self(fn):
"""Does this body call a method ON the record -- rec._helper(), self._go()?
The read is then inside the helper, where this pass cannot follow it, and a
dependency declared for the helper's benefit is correct. Reporting it would
be telling the author to delete a line that is doing its job.
Length one, deliberately. rec._helper() resolves to the path ('_helper',)
and can read anything; rec.line_ids.filtered(...) resolves to ('line_ids',
'filtered') and is an ORM call on one field, which reaches no other. Taking
both lost the incident this checker was written from, where the body's only
call was .replace() on a string.
"""
aliases = _aliases(fn)
for node in _walk_body(fn):
if not isinstance(node, ast.Call):
continue
func = node.func
if not isinstance(func, ast.Attribute):
continue
path = _attr_path(func, aliases)
if path is not None and len(path) == 1:
return True
return False
# ------------------------------------------------ evidence for the triggers
#
# Two rules in the corpus are conditional on something no single file contains.
# `ondelete` only bites if the parent is ever actually deleted; `check_company`
# only bites if the codebase runs multi-company at all. Both conditions are
# answerable from the tree, and these are the per-file halves of that answer --
# context.tree_facts aggregates them and the checkers read the aggregate.
#
# Everything here returns EVIDENCE FOUND, never evidence absent. One file not
# deleting a model says nothing; the whole tree not deleting it is a claim, and
# the only place entitled to make it is the thing that saw every file.
# How a codebase shows that more than one company is really in play. A plain
# `company_id` is not on this list on purpose: nearly every Odoo model declares
# one, in single- and multi-company deployments alike, so it separates nothing.
# These four are the machinery you only write when several companies exist --
# a user belonging to more than one, code reading the active set, or an author
# who has already met the rule.
MULTI_COMPANY_MARKERS = ("company_ids", "allowed_company_ids",
"env.companies", "check_company")
def models_declared(code):
"""Model names this file DEFINES, from `_name`. Not `_inherit`.
The distinction decides what a tree is entitled to say about deletion.
A model defined here is one whose code and access rules this tree owns, so
"nothing in the tree deletes it" is a claim about the whole of it. A model
merely extended with `_inherit` is somebody else's -- core Odoo's, usually --
and the delete could be anywhere in a codebase this walk never saw.
"""
tree, _ = parse_python(code)
if tree is None:
return set()
names = set()
for cls in [n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)]:
for node in cls.body:
if not isinstance(node, ast.Assign) or len(node.targets) != 1:
continue
target = node.targets[0]
if not (isinstance(target, ast.Name) and target.id == "_name"):
continue
if (isinstance(node.value, ast.Constant)
and isinstance(node.value.value, str)):
names.add(node.value.value)
return names
# Field types whose first positional argument names the comodel. For every other
# type the first positional argument is the label.
RELATIONAL_FIELDS = frozenset(["Many2one", "One2many", "Many2many"])
def orm_declarations(code):
"""What a file declares about its models, for the tree facts.
One record per class that names a model: the model, the models it inherits
(`_inherit` beyond itself, which put their methods in its MRO), the fields
it declares and the literal @api.depends of each method that has one.
Read the way Odoo reads them (odoo/models.py, 18.0 at 3c3e3b3): `_name`,
or failing that a single `_inherit`; a class inheriting several models
without a `_name` is named after itself there, and is left out here.
A field is {"compute", "related", "store", "inverse", "comodel"}. `store`
is True, False, None when the declaration does not say, or "?" when it
says it with something other than a literal. A method's depends are the
list, or None when @api.depends is there and cannot be read. `named` says
the class gives the model its `_name`, rather than extending it.
"""
# ast.parse alone: the compile step parse_python adds reports errors this
# never reads, and took about a quarter of the time (ARCHITECTURE 3.84).
try:
tree = ast.parse(code)
except SyntaxError:
return []
records = []
for cls in [n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)]:
model, named, inherit = _class_model(cls)
if model is None:
continue
fields, methods = {}, {}
for field, call in _class_field_calls(cls).items():
keywords = {k.arg: k.value for k in call.keywords if k.arg}
store = keywords.get("store")
compute = keywords.get("compute")
comodel = None
if call.func.attr in RELATIONAL_FIELDS:
comodel = (_literal_string(call.args[0]) if call.args
else _literal_string(keywords.get("comodel_name")))
fields[field] = {
"compute": (compute.value if isinstance(compute, ast.Constant)
and isinstance(compute.value, str)
else compute.id if isinstance(compute, ast.Name) else None),
"related": _literal_string(keywords.get("related")),
"store": (None if store is None else store.value
if isinstance(store, ast.Constant) else "?"),
"inverse": "inverse" in keywords,
"comodel": comodel,
}
for fn in cls.body:
if not isinstance(fn, (ast.FunctionDef, ast.AsyncFunctionDef)):
continue
depends = _decorator_args(fn, "api", "depends")
if depends is not None:
methods[fn.name] = None if depends is UNREADABLE else list(depends)
records.append({"model": model, "named": named,
"parents": [p for p in inherit if p != model],
"fields": fields, "methods": methods})
return records
def _literal_string(node):
return node.value if isinstance(node, ast.Constant) and isinstance(node.value, str) \
else None
def _literal_strings(node):
"""['a', 'b'] for "a" or ["a", "b"]; None when any part is not a literal."""
single = _literal_string(node)
if single is not None:
return [single]
if isinstance(node, (ast.List, ast.Tuple)):
values = [_literal_string(element) for element in node.elts]
return values if all(v is not None for v in values) else None
return None
def models_unlinked(code):
"""Model names this file gives evidence of DELETING.
Three shapes, which between them are how a delete is spelled in practice:
self.env['x.y'].search(...).unlink() the model is named in the call
self.unlink() inside a class, so the class's own
models are what gets deleted
def unlink(self) an override. Nobody writes one for a
model nothing deletes, and it is the
single clearest statement that
deletion is a path this code expects.
A local name assigned from `self.env['x.y']` and unlinked later resolves too,
because splitting the search and the unlink across two lines is the common
spelling and a checker that missed it would call the model undeleted.
"""
tree, _ = parse_python(code)
if tree is None:
return set()
owners = {} # class node -> the models it speaks for
for cls in [n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)]:
owners[cls] = _model_names(cls)
found = set()
# An override is a statement about the model the class declares.
for cls, names in owners.items():
for node in cls.body:
if (isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
and node.name == "unlink"):
found |= names
# Local names that hold a recordset of a known model.
bound = {}
for node in ast.walk(tree):
if (isinstance(node, ast.Assign) and len(node.targets) == 1
and isinstance(node.targets[0], ast.Name)):
model = _env_model_in(node.value)
if model:
bound[node.targets[0].id] = model
for node in ast.walk(tree):
if not (isinstance(node, ast.Call)
and isinstance(node.func, ast.Attribute)
and node.func.attr == "unlink"):
continue
receiver = node.func.value
model = _env_model_in(receiver)
if model:
found.add(model)
continue
if isinstance(receiver, ast.Name) and receiver.id in bound:
found.add(bound[receiver.id])
continue
# `self.unlink()` and `record.unlink()` inside a model body: the models
# the enclosing class declares are the ones being deleted.
if isinstance(receiver, ast.Name) and receiver.id == "self":
found |= _enclosing_models(tree, node, owners)
return found
def _env_model_in(node):
"""The model name in an `env['x.y']` subscript anywhere inside node, if the
expression is rooted in one. `self.env['x'].search(...)` and
`self.env['x'].browse(i)` both resolve; a bare `records` does not."""
if node is None:
return None
for inner in ast.walk(node):
if not isinstance(inner, ast.Subscript):
continue
value = inner.value
if not (isinstance(value, ast.Attribute) and value.attr == "env"):
continue
key = inner.slice
if isinstance(key, ast.Constant) and isinstance(key.value, str):
return key.value
return None
def _enclosing_models(tree, target, owners):
"""The models declared by the class that lexically contains `target`."""
for cls, names in owners.items():
for node in ast.walk(cls):
if node is target:
return names
return set()
def declares_multi_company(code):
"""True if this file shows the codebase really runs more than one company.
Textual on purpose. The markers are attribute names and a keyword argument
rather than a shape, so an AST pass would spell the same search at more
length, and the question is only ever "does this appear anywhere".
"""
view = strip_comments(code, "python")
return any(marker in view for marker in MULTI_COMPANY_MARKERS)
# ------------------------------------------- where a value came from
#
# Four rules were classified false for the same reason: each matched a
# variable's NAME and called that evidence about its value. `total == 0` is a
# float comparison in one function and an exact integer test in the next;
# `filename` says nothing about whether a caller chose it. Telling those apart
# needs the assignment, which is what this section reads.
#
# It is a local trace and nothing more. One file, one function, no imports
# followed and no types inferred -- which is enough for the shapes that were
# actually misread, and the limit is stated here rather than discovered later.
def _function_of(tree):
"""Maps every node to the function that encloses it, innermost winning.
Lambdas count. `filtered(lambda x: x.qty == 0)` is where one of the
misreadings lived, and a lambda binds its arguments exactly as a def does.
"""
owner = {}
scopes = [n for n in ast.walk(tree)
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda))]
# Outermost first, so an inner scope overwrites the outer one's claim.
scopes.sort(key=lambda n: (getattr(n, "lineno", 0),
getattr(n, "col_offset", 0)))
for fn in scopes:
for node in ast.walk(fn):
owner[id(node)] = fn
return owner
def _final_name(node):
"""The last identifier in a Name or Attribute, or None for anything else."""
if isinstance(node, ast.Name):
return node.id
if isinstance(node, ast.Attribute):
return node.attr
return None
def _chain_root(node):
"""The leftmost Name in a chain of attributes, calls and subscripts."""
while True:
if isinstance(node, ast.Attribute):
node = node.value
elif isinstance(node, ast.Subscript):
node = node.value
elif isinstance(node, ast.Call):
node = node.func
else:
break
return node.id if isinstance(node, ast.Name) else None
def _targets(node):
"""Every Name a statement binds, whatever the binding form."""
found = []
for sub in ast.walk(node):
if isinstance(sub, ast.Name) and isinstance(sub.ctx, (ast.Store, ast.Del)):
found.append(sub.id)
return found
def _bound_in(fn):
"""Names this scope binds: parameters, assignments, loops, withs, imports."""
if fn is None:
return set()
names = set()
args = getattr(fn, "args", None)
if args is not None:
for group in (args.args, args.posonlyargs, args.kwonlyargs):
names.update(a.arg for a in group or [])
for extra in (args.vararg, args.kwarg):
if extra is not None:
names.add(extra.arg)
for node in ast.walk(fn):
if isinstance(node, (ast.Assign, ast.AnnAssign, ast.AugAssign, ast.For,
ast.AsyncFor, ast.comprehension, ast.NamedExpr)):
names.update(_targets(node))
elif isinstance(node, (ast.With, ast.AsyncWith)):
for item in node.items:
if item.optional_vars is not None:
names.update(_targets(item.optional_vars))
elif isinstance(node, ast.ExceptHandler) and node.name:
names.add(node.name)
elif isinstance(node, (ast.Import, ast.ImportFrom)):
names.update((a.asname or a.name).split(".")[0] for a in node.names)
elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
if node is not fn:
names.add(node.name)
return names
def _bound_at_module(tree):
"""Names the file itself binds at any level outside a function body."""
names = set()
for node in ast.walk(tree):
if isinstance(node, (ast.Assign, ast.AnnAssign, ast.AugAssign)):
names.update(_targets(node))
elif isinstance(node, (ast.Import, ast.ImportFrom)):
names.update((a.asname or a.name).split(".")[0] for a in node.names)
elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
names.add(node.name)
return names
# Reads that are external by their own name, whatever the surrounding file
# does with them. `request` is werkzeug's, reached through odoo.http; the
# context is whatever the caller put there; both parsers turn somebody else's
# bytes into objects.
EXTERNAL_ROOTS = frozenset(["request"])
EXTERNAL_CALLS = frozenset(["loads", "literal_eval", "parse_qs", "parse_qsl"])
CONTEXT_READS = ("_context", "context")
def _is_external(node, fn, tree):
"""True when this expression's value came from outside the file.
Two ways to qualify. Either the expression names a source that is external
by construction -- `request`, a parsed payload, the context a caller
handed in -- or its root name is one this file never binds, which is the
same answer arrived at by elimination: a name that is not a parameter, not
assigned, not a loop or with target, not imported and not a builtin is a
value from somewhere this file cannot see.
The elimination half is what makes the rules' own 'wrong' samples fire,
because a sample is a fragment and its inputs are free names by
definition. It is also what keeps them quiet on the four findings that
were classified false: `file_config`, `filename`, `full_path` and
`raw_value` are all bound a few lines above the join.
"""
for sub in ast.walk(node):
if isinstance(sub, ast.Attribute) and sub.attr in CONTEXT_READS:
return True
if isinstance(sub, ast.Call):
attr = _final_name(sub.func)
if attr in EXTERNAL_CALLS:
return True
root = _chain_root(node)
if root is None:
return False
if root in EXTERNAL_ROOTS:
return True
if root in ("self", "cls"):
return False
if root in dir(builtins):
return False
return root not in _bound_in(fn) and root not in _bound_at_module(tree)
# Calls whose result is a whole number, so `== 0` on it is exact and the
# float rule has nothing to say. search_count and len are counts by
# definition; `.count()` is one on a recordset, a string and a list alike.
INT_VALUED_CALLS = frozenset(["search_count", "count", "len", "bit_length",
"index", "ord"])
_COUNT_KEY = re.compile(r"(?:^__count$|_count$)")
def _integer_fields(tree):
"""Field names the file declares as fields.Integer."""
names = set()
for node in ast.walk(tree):
if not isinstance(node, ast.Assign) or not isinstance(node.value, ast.Call):
continue
func = node.value.func
if isinstance(func, ast.Attribute) and func.attr == "Integer":
names.update(_targets(node))
return names
def _counts_rows(fn):
"""True when this scope runs a COUNT query, so a fetched cell is an int."""
if fn is None:
return False
for node in ast.walk(fn):
if not (isinstance(node, ast.Call) and _final_name(node.func) == "execute"):
continue
for arg in node.args[:1]:
try:
text = ast.unparse(arg)
except Exception: # pragma: no cover
continue
if "count(" in text.lower():
return True
return False
def _assigned_values(fn, name):
"""Values assigned to `name` in this scope, or None if it is bound some
way the trace cannot read -- a loop target, a with, an unpacking."""
if fn is None:
return None
values = []
for node in ast.walk(fn):
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == name:
values.append(node.value)
elif name in _targets(target):
return None # unpacked: no single value
elif isinstance(node, ast.AnnAssign):
if isinstance(node.target, ast.Name) and node.target.id == name \
and node.value is not None:
values.append(node.value)
elif isinstance(node, (ast.For, ast.AsyncFor, ast.comprehension)):
if name in _targets(node.target):
return None
elif isinstance(node, (ast.With, ast.AsyncWith)):
for item in node.items:
if item.optional_vars is not None \
and name in _targets(item.optional_vars):
return None
elif isinstance(node, ast.AugAssign):
if isinstance(node.target, ast.Name) and node.target.id == name:
values.append(node.value)
return values or None
def _int_valued(node, fn, tree, depth=0):
"""True when this expression is certainly a whole number.
Certainly, not probably: every answer here SUPPRESSES a finding, so the
unreadable cases must come back False and leave the finding standing. A
name assigned twice is only an integer if both assignments are.
"""
if depth > 4: # pragma: no cover
return False
if isinstance(node, ast.Constant):
return isinstance(node.value, int) and not isinstance(node.value, bool)
if isinstance(node, ast.Call):
if _final_name(node.func) in INT_VALUED_CALLS:
return True
return False
if isinstance(node, ast.Subscript):
# read_group hands back rows keyed '<field>_count', and the key is
# written as an f-string as often as a literal.
key = node.slice
if isinstance(key, ast.Constant) and isinstance(key.value, str):
if _COUNT_KEY.search(key.value):
return True
if isinstance(key, ast.JoinedStr) and key.values:
tail = key.values[-1]
if isinstance(tail, ast.Constant) and isinstance(tail.value, str) \
and _COUNT_KEY.search(tail.value):
return True
# cr.fetchone()[0] of a SELECT COUNT(*) is the third spelling of a
# count, and the query is the only place that says so.
inner = node.value
if isinstance(inner, ast.Call) and _final_name(inner.func) in (
"fetchone", "fetchall") and _counts_rows(fn):
return True
return False
if isinstance(node, ast.BinOp) and isinstance(node.op, (ast.Add, ast.Sub,
ast.Mult, ast.Mod,
ast.FloorDiv)):
return (_int_valued(node.left, fn, tree, depth + 1)
and _int_valued(node.right, fn, tree, depth + 1))
if isinstance(node, ast.Attribute):
return node.attr in _integer_fields(tree)
if isinstance(node, ast.Name):
if node.id in _integer_fields(tree):
return True
values = _assigned_values(fn, node.id)
if not values:
return False
return all(_int_valued(v, fn, tree, depth + 1) for v in values)
return False
# ------------------------------------------------ the rewritten checkers
_MONEY_NAME = re.compile(r"price|amount|qty|quantity|total|balance", re.I)
_RESOLVES_THE_PATH = frozenset(["commonpath", "commonprefix", "relpath",
"is_relative_to", "realpath", "abspath"])
def float_equality_against_zero(code):
"""odoo.float-comparison-needs-float-utils
`== 0` or `!= 0` on a value whose name says it is money or a quantity,
where the assignment does not say it is a whole number.
The regex this replaces asked only the first half. Classified by hand it
was 2 true and 5 false, and every one of the five compared an INTEGER to
zero, where `==` is exact and the rule has nothing to say:
`search_count(...)`, a read_group `<field>_count`, `cr.fetchone()[0]` of a
SELECT COUNT(*), and `x.qty` where the same file declares qty as
fields.Integer. All five were named `total`, `group_total` or
`total_population`, and so were both true positives -- `total =
sum(sorted_amounts)` is the same spelling with a float behind it. The name
could never have separated them.
What it cannot see, and so still reports: a float that arrives as a
parameter, and an integer from a helper in another file. Both leave the
finding standing, which is the direction a suppressing trace has to err in.
"""
tree, _ = parse_python(code)
if tree is None:
return []
owner = _function_of(tree)
findings, seen = [], set()
for node in ast.walk(tree):
if not isinstance(node, ast.Compare) or len(node.ops) != 1:
continue
if not isinstance(node.ops[0], (ast.Eq, ast.NotEq)):
continue
left, right = node.left, node.comparators[0]
for value, other in ((left, right), (right, left)):
if not (isinstance(other, ast.Constant)
and isinstance(other.value, (int, float))
and not isinstance(other.value, bool)
and other.value == 0):
continue
name = _final_name(value)
if not name or not _MONEY_NAME.search(name):
continue
if _int_valued(value, owner.get(id(value)), tree):
break
if node.lineno in seen:
break
seen.add(node.lineno)
findings.append((
node.lineno,
"'{0}' is compared with == against zero -- binary floats do "
"not land on it exactly, so use float_is_zero with the "
"field's precision".format(name),
None))
break
return findings
def path_join_takes_an_external_name(code):
"""python.path-join-does-not-contain-a-path
os.path.join where one component came from outside the file, and the
result is not resolved and compared before it is used.
The regex this replaces matched component NAMES -- `filename`, `form`,
`params` -- and was 0 true, 4 false. All four join a configured directory
to a name the module itself produced: a timestamped backup filename, a
literal from a list declared in the same function, a module path out of
get_module_path. Not one was a fragment a caller could point out of the
directory, which is the only thing the rule is about.
So the question moved from what the component is called to where its value
came from, and 'outside the file' is the answer _is_external gives. The
rule's own wrong sample joins `request.args['name']` and still fires.
This reports less than the regex did, on purpose: the regex's entire
output on 294,000 lines was false, and a checker that cries wolf gets
switched off. A traversal reached through a helper in another module is
outside what a local trace can see and is not reported.
"""
tree, _ = parse_python(code)
if tree is None:
return []
owner = _function_of(tree)
findings = []
for node in ast.walk(tree):
if not (isinstance(node, ast.Call) and _final_name(node.func) == "join"):
continue
# os.path.join, not str.join and not pathlib's.
parent = node.func.value if isinstance(node.func, ast.Attribute) else None
if not (isinstance(parent, ast.Attribute) and parent.attr == "path"):
continue
fn = owner.get(id(node))
# The fix is to resolve and compare, and it is written beside the
# join. Scoped to the enclosing function rather than the file: a
# realpath somewhere else in a 900-line model says nothing about this
# call. A join at module scope -- which is what a rule's own sample
# is -- is scoped to the module for the same reason.
scope = fn if fn is not None else tree
if any(_final_name(c.func) in _RESOLVES_THE_PATH
for c in ast.walk(scope) if isinstance(c, ast.Call)):
continue
for arg in node.args[1:]:
if _is_external(arg, fn, tree):
try:
shown = ast.unparse(arg)
except Exception: # pragma: no cover
shown = "the component"
findings.append((
node.lineno,
"os.path.join is handed {0}, which came from outside this "
"file -- an absolute component discards the base directory "
"entirely and '..' walks out of it, so resolve both and "
"compare before opening anything".format(shown),
None))
break
return findings
# Modules whose presence in the imports makes a file an HTTP edge. Not
# werkzeug: Odoo imports werkzeug.urls to build links in model files, and one
# such file on core was the only thing a wider gate added.
_HTTP_MODULES = ("fastapi", "pydantic", "odoo.http", "starlette")
def _datetime_class_names(tree):
"""Names the datetime CLASS is bound to: `datetime`, plus any alias."""
names = {"datetime"}
for node in ast.walk(tree):
if isinstance(node, ast.ImportFrom) and node.module == "datetime":
names.update(a.asname for a in node.names
if a.name == "datetime" and a.asname)
return names
def _speaks_http(tree):
for node in ast.walk(tree):
if isinstance(node, ast.ImportFrom) and node.module:
if node.module.startswith(_HTTP_MODULES):
return True
if node.module == "odoo" and any(a.name == "http" for a in node.names):
return True
elif isinstance(node, ast.Import):
if any(a.name.startswith(_HTTP_MODULES) for a in node.names):
return True
return False
class _NaiveNow(object):
"""Which expressions carry a naive now(), and as a time or as a date.
The sources are the ones the regex matched -- datetime.now() and utcnow()
with empty parentheses -- and nothing more. date.today() and
fields.Date.today() are the same UTC date and were measured rather than
added: 48 and 114 sites on OpenSPP, 31 and 217 on core. Adding them would
turn a narrowing into a widening, and fields.Date.today() is the
framework's own spelling of the value.
"""
def __init__(self, tree):
self.names = _datetime_class_names(tree)
self.owner = _function_of(tree)
self.at = None
def _is_source(self, node):
if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)):
return False
if node.args or node.keywords:
return False
if node.func.attr == "utcnow":
return True
return node.func.attr == "now" and _final_name(node.func.value) in self.names
def carries(self, node, depth=0):
"""None, "datetime" or "date"; the source's line is left in self.at."""
if node is None or depth > 5:
return None
if self._is_source(node):
self.at = node.lineno
return "datetime"
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute):
inner = self.carries(node.func.value, depth + 1)
if node.func.attr == "date" and not node.args and inner:
return "date"
if node.func.attr == "replace" and inner:
# .replace(tzinfo=...) is the fix; any other replace keeps it naive.
return None if any(k.arg == "tzinfo" for k in node.keywords) else inner
if node.func.attr == "get" and len(node.args) == 2:
# vals.get("date", datetime.now()): the default is the value
# whenever the caller did not supply one.
return self.carries(node.args[1], depth + 1)
return None
if isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add):
return self.carries(node.left, depth + 1) or self.carries(node.right, depth + 1)
if isinstance(node, ast.BinOp) and isinstance(node.op, ast.Sub):
# now - timedelta is a time; now - then is a duration.
left = self.carries(node.left, depth + 1)
return None if self.carries(node.right, depth + 1) else left
if isinstance(node, (ast.BoolOp, ast.IfExp)):
parts = node.values if isinstance(node, ast.BoolOp) else [node.body, node.orelse]
for part in parts:
found = self.carries(part, depth + 1)
if found:
return found
return None
if isinstance(node, ast.Name):
for value in _assigned_values(self.owner.get(id(node)), node.id) or ():
found = self.carries(value, depth + 1)
if found:
return found
return None
def serialised(self, node):
"""`x.isoformat()` or `str(x)` of a naive value: text with no offset."""
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and node.func.attr == "isoformat" and not node.args:
return self.carries(node.func.value)
if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) \
and node.func.id == "str" and node.args:
return self.carries(node.args[0])
return None
def _offset_written_by_hand(parent_of, node):
"""`f"{now.isoformat()}Z"` or `now.isoformat() + "Z"`: labelled UTC."""
up = parent_of.get(id(node))
while isinstance(up, ast.FormattedValue):
up = parent_of.get(id(up))
tails = []
if isinstance(up, ast.JoinedStr):
tails = [v.value for v in up.values if isinstance(v, ast.Constant)]
elif isinstance(up, ast.BinOp) and isinstance(up.op, ast.Add) \
and isinstance(up.right, ast.Constant):
tails = [up.right.value]
return any(isinstance(t, str) and t.startswith(("Z", "+00")) for t in tails)
def _names_a_date(key):
"""A vals key or domain field spelled as a date: date, date_*, *_date."""
if not (isinstance(key, ast.Constant) and isinstance(key.value, str)):
return False
k = key.value
return "datetime" not in k and (k == "date" or k.startswith("date_")
or k.endswith("_date"))
def naive_time_leaves_or_dates_a_day(code):
"""python.naive-and-aware-datetimes-do-not-compare
The two shapes that survived reading, and only those.
The regex this replaces fired on every datetime.now() and utcnow(). Its
premise was that a naive local time reaching the ORM is wrong by the
server's offset, and inside Odoo there is no offset: `import odoo` pins
TZ=UTC, and fields.Datetime.now() is datetime.now() itself (ARCHITECTURE
3.56). Re-read on that basis it was 3 true of 33 on OpenSPP and OCA and 0
of 15 sampled on core. What stands does not depend on the server clock:
A. A naive timestamp LEAVING the system with no offset: its isoformat()
anywhere, or the value itself as a keyword of a class being built (a
pydantic schema, a response object) or in a dict the function
returns, in a file that imports fastapi, pydantic, starlette or
odoo.http. The consumer cannot tell the zone. Two consent API
responses on OpenSPP, both commits recall credits the rule with, and
the xades:SigningTime of two Spanish e-invoicing modules on core.
B. The server's UTC DATE used as a calendar day: a .date() of it into a
vals key or domain field spelled as a date, a Date field's default,
or an attribute named for a date; and a naive now() itself into a key
named exactly `date`, which Odoo truncates to UTC's day. OCA's
purchase_unreconciled dates a write-off that way, so for a company
ahead of UTC the early hours land on the day before.
What it leaves alone on purpose: a comparison (the fiscal-year cron was a
day ahead by design), a filename or label (display only), a JWT payload
(PyJWT reads naive as UTC), an ISO string with a Z written after it, and a
value that only meets other naive UTC values inside the ORM. A key ending
in _date can be a Datetime field (stock's scheduled_date), which is why a
naive now() into one is not reported and its .date() is -- and why
l10n_in's demo invoices, `'invoice_date': datetime.now()` into a Date
field, are missed. The isoformat() clause costs three findings OpenSPP's
classification reads false: a generated-at line in two report scripts and
a computed_at in a result dict.
"""
tree, _ = parse_python(code)
if tree is None:
return []
now = _NaiveNow(tree)
parent_of = {}
for node in ast.walk(tree):
for child in ast.iter_child_nodes(node):
parent_of[id(child)] = node
returned = set()
for fn in _functions(tree):
for node in ast.walk(fn):
if isinstance(node, ast.Return) and node.value is not None:
returned.add(id(node.value))
if isinstance(node.value, ast.Name):
returned.update(id(v) for v in _assigned_values(fn, node.value.id) or ())
findings, seen = [], set()
def report(text):
if now.at not in seen:
seen.add(now.at)
findings.append((now.at, text, None))
# An ISO string is text for somebody else to read, wherever it is built.
# The HTTP gate alone lost two on core: the Spanish e-invoicing modules
# write datetime.now().isoformat() into xades:SigningTime, an xsd:dateTime
# that a reader with no offset takes as local time.
for node in ast.walk(tree):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and node.func.attr == "isoformat" and now.serialised(node) \
and not _offset_written_by_hand(parent_of, node):
report("isoformat() of a naive timestamp writes no offset, so whoever "
"reads the text cannot tell which zone it is in -- write "
"datetime.now(timezone.utc) at the edge")
if _speaks_http(tree):
for node in ast.walk(tree):
if isinstance(node, ast.Call) and (_final_name(node.func) or "")[:1].isupper():
values = [k.value for k in node.keywords]
where = "into {0}()".format(_final_name(node.func))
elif isinstance(node, ast.Dict) and id(node) in returned:
values = [v for v in node.values if v is not None]
where = "in a returned dict"
else:
continue
for value in values:
if (now.carries(value) or now.serialised(value)) \
and not _offset_written_by_hand(parent_of, value):
report("a naive timestamp leaves {0} with no offset, so whoever "
"reads it cannot tell which zone it is in -- write "
"datetime.now(timezone.utc) at the edge".format(where))
for node in ast.walk(tree):
if isinstance(node, ast.Dict):
for key, value in zip(node.keys, node.values):
if not _names_a_date(key):
continue
kind = now.carries(value)
if kind == "date" or (kind == "datetime" and key.value == "date"):
report("'{0}' is set to the server's UTC date -- for a company "
"ahead of UTC that is yesterday in its early hours; "
"fields.Date.context_today is the user's day".format(key.value))
elif isinstance(node, ast.Tuple) and len(node.elts) == 3 \
and _names_a_date(node.elts[0]):
if now.carries(node.elts[2]) == "date":
report("a domain on '{0}' compares against the server's UTC date "
"rather than the user's -- fields.Date.context_today is "
"the user's day".format(node.elts[0].value))
elif isinstance(node, ast.Call) and _final_name(node.func) == "Date":
for kw in node.keywords:
body = kw.value.body if isinstance(kw.value, ast.Lambda) else kw.value
if kw.arg == "default" and now.carries(body):
report("a Date field defaults to the server's UTC date -- "
"fields.Date.context_today is the user's day")
elif isinstance(node, ast.Assign) and len(node.targets) == 1 \
and isinstance(node.targets[0], ast.Attribute):
attr = node.targets[0].attr
if "datetime" not in attr and "date" in attr.split("_") \
and now.carries(node.value) == "date":
report("'{0}' is set to the server's UTC date -- "
"fields.Date.context_today is the user's day".format(attr))
return sorted(findings)
def float_coercion_of_external_money(code):
"""python.decimal-for-money-not-float
float() applied to a money value that arrived from outside the file.
The regex this replaces matched the NAME either side of the coercion and
was 0 true, 3 false, with a reason behind it rather than a sample size:
Odoo's Monetary field IS a float -- the ORM defines it that way -- and the
framework's answer to the precision problem is float_round and
float_compare against the currency's rounding, not Decimal. Inside an Odoo
module the rule fired on the idiom and named a fix that cannot be taken.
Three findings do not settle a rule either way, so it is narrowed rather
than culled, and the narrowing is the line the argument actually draws.
`float(self.amount or 0.0)` is the ORM's own type and is not reported.
`float(result)`, where result came back from a service in this file, is
not reported. `float(payload['price'])`, where the string came from a
caller and Decimal(payload['price']) IS available, still is -- which is
the rule's own wrong sample and the one shape its remedy fits.
"""
tree, _ = parse_python(code)
if tree is None:
return []
owner = _function_of(tree)
findings = []
for node in ast.walk(tree):
if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Name)
and node.func.id == "float" and node.args):
continue
value = node.args[0]
name = _final_name(value) or _chain_root(value) or ""
target = _MONEY_NAME.search(name)
if not target:
# The coerced expression may say nothing, while what it is
# assigned to does: `price = float(payload['p'])`.
holder = owner.get(id(node))
for stmt in ast.walk(holder if holder is not None else tree):
if isinstance(stmt, ast.Assign) and stmt.value is node:
target = any(_MONEY_NAME.search(t)
for t in _targets(stmt) if t)
break
if not target:
continue
if not _is_external(value, owner.get(id(node)), tree):
continue
findings.append((
node.lineno,
"a money value from outside this file is routed through float() "
"-- the binary float is already wrong before Decimal can be given "
"it, so build Decimal from the original string",
None))
return findings
class _Every(object):
"""A constraint is present and which fields it covers cannot be read."""
EVERY_FIELD = _Every()
def _sql_identifiers(node):
"""Names inside a constraint's SQL text, or EVERY_FIELD when the text is
not a literal. Keywords come along (CHECK, UNIQUE, NOT) and match no field."""
if isinstance(node, ast.Constant) and isinstance(node.value, str):
return set(re.findall(r"[A-Za-z_][A-Za-z0-9_]*", node.value))
return EVERY_FIELD
def _constrained_fields(tree, legacy_dead):
"""Field names some constraint in this file covers, or EVERY_FIELD.
Three spellings: @api.constrains (its plain names -- a dotted one registers
nothing), models.Constraint (the names in its SQL) and _sql_constraints (the
names in each entry's SQL, and nothing at all when `legacy_dead`, because
from 19 the registry never creates it). A constraint that is present but
cannot be read -- @api.constrains(*FIELDS), a _sql_constraints inherited and
extended -- covers every field, which is the reading this had before it
looked at fields at all.
"""
covered = set()
def add(names):
if names is EVERY_FIELD:
return True
covered.update(names)
return False
for fn in _functions(tree):
declared = _decorator_args(fn, "api", "constrains")
if declared is UNREADABLE:
return EVERY_FIELD
if declared:
covered.update(d for d in declared if "." not in d)
for node in ast.walk(tree):
if isinstance(node, ast.Call):
func = node.func
name = func.attr if isinstance(func, ast.Attribute) else \
func.id if isinstance(func, ast.Name) else None
if name == "Constraint":
if add(_sql_identifiers(node.args[0]) if node.args else EVERY_FIELD):
return EVERY_FIELD
continue
if isinstance(node, (ast.Assign, ast.AugAssign, ast.AnnAssign)):
targets = node.targets if isinstance(node, ast.Assign) else [node.target]
if not any(isinstance(t, ast.Name) and t.id == "_sql_constraints"
for t in targets) or node.value is None or legacy_dead:
continue
if not isinstance(node.value, (ast.List, ast.Tuple)):
return EVERY_FIELD
for entry in node.value.elts:
if not (isinstance(entry, (ast.List, ast.Tuple)) and len(entry.elts) > 1):
return EVERY_FIELD
if add(_sql_identifiers(entry.elts[1])):
return EVERY_FIELD
return covered
def _clears(value):
"""False, None, '' or an empty collection: what a form resets a field to."""
if isinstance(value, ast.Constant):
return value.value is False or value.value is None or value.value == ""
if isinstance(value, ast.Dict):
return not value.keys
return isinstance(value, (ast.List, ast.Tuple, ast.Set)) and not value.elts
def _refuses_in_branch(fn, warning, watched):
"""Does the branch that returns this warning also take the value back?
A warning dict on its own informs: "this reference already exists", "the
rate is far from the previous one", "please choose another package type".
The user saves anyway, from the form as from an import, so nothing is
enforced anywhere and the rule's harm -- a check only the form applies --
is not there. What refuses is the same branch writing back a field the
onchange watches (the quantity clamped, the partner cleared, the removed
document restored) or clearing any field on the record (a registrant that
does not fit the new request type). "The same branch" is the innermost
if, for or while around the warning, or the whole body when there is none.
"""
parents = {}
for node in ast.walk(fn):
for child in ast.iter_child_nodes(node):
parents[id(child)] = node
branch = warning
while branch is not fn and not isinstance(branch, (ast.If, ast.For, ast.AsyncFor,
ast.While)):
branch = parents.get(id(branch), fn)
roots = set() if watched is UNREADABLE else {w.split(".")[0] for w in watched}
aliases = _aliases(fn)
def on_record(node):
return isinstance(node, ast.Name) and aliases.get(node.id) == ()
for node in ast.walk(branch):
if isinstance(node, (ast.Assign, ast.AugAssign)):
targets = node.targets if isinstance(node, ast.Assign) else [node.target]
for target in targets:
if isinstance(target, ast.Attribute) and on_record(target.value) and (
watched is UNREADABLE or target.attr in roots
or _clears(node.value)):
return True
elif (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)
and node.func.attr == "update" and on_record(node.func.value)
and node.args and isinstance(node.args[0], ast.Dict)):
for key, value in zip(node.args[0].keys, node.args[0].values):
if isinstance(key, ast.Constant) and (
watched is UNREADABLE or key.value in roots or _clears(value)):
return True
return False
def _raised_messages(scope):
"""The string literals inside every raise in this scope, long enough to
name one check: `_()` and implicit concatenation are already one string."""
messages = set()
for node in ast.walk(scope):
if isinstance(node, ast.Raise):
for inner in ast.walk(node):
if isinstance(inner, ast.Constant) and isinstance(inner.value, str) \
and len(inner.value.strip()) >= 20:
messages.add(inner.value.strip())
return messages
def _check_helpers(scope, fields=None):
"""Names of the check-shaped methods this scope calls: _check_expiration_date,
_validate_dates. A name is all there is to go on, so only those two words.
With `fields`, only a call handed one of those fields counts:
_check_expiration_date(self.expiration_date) checks the watched value,
and a bare self._check_company() in the same onchange checks something
else that write() may also happen to check.
"""
names = set()
for node in ast.walk(scope):
if not isinstance(node, ast.Call):
continue
name = _final_name(node.func)
if not (name and ("check" in name.lower() or "validat" in name.lower())):
continue
if fields is not None and not any(
isinstance(arg, ast.Attribute) and arg.attr in fields
for arg in list(node.args) + [kw.value for kw in node.keywords]):
continue
names.add(name)
return names
def _server_methods(cls, fn):
"""Methods of this class the server runs whatever the client is: create(),
write(), an @api.constrains method, and any method a field in the class
names as its inverse=. Not action_post and friends: a check at posting
lets a draft hold the value, which is the gap the rule is about."""
inverses = set()
for node in ast.walk(cls):
if isinstance(node, ast.Call):
for keyword in node.keywords:
if keyword.arg == "inverse" and isinstance(keyword.value, ast.Constant):
inverses.add(keyword.value.value)
return [m for m in cls.body
if isinstance(m, (ast.FunctionDef, ast.AsyncFunctionDef)) and m is not fn
and (m.name in ("create", "write") or m.name in inverses
or _decorator_args(m, "api", "constrains") is not None)]
def _server_repeats_the_check(tree, fn, watched):
"""Does a server-side method of the same class make the onchange's check?
Two signals, and both have to be the SAME check rather than a similar one.
The exact message the onchange raises, raised again: mrp.workorder's
date_finished onchange refuses to unplan one work order, and _set_dates,
the inverse of both date fields, raises the same sentence. Or the same
check-shaped helper called from both: the API key wizard's onchange and
its create() both call res.users.apikeys._check_expiration_date.
Only the onchange's own class is searched, one call deep, because every
yes here suppresses a finding. A check made at posting does not count
(see _server_methods), and a paraphrased message is not the same check.
"""
cls = _owner_of(tree, fn)
if cls is None or watched is UNREADABLE:
return False
messages = _raised_messages(fn)
helpers = _check_helpers(fn, {w.split(".")[0] for w in watched})
if not messages and not helpers:
return False
for method in _server_methods(cls, fn):
if messages & _raised_messages(method) or helpers & _check_helpers(method):
return True
return False
def onchange_tries_to_enforce(code, facts=None):
"""odoo.onchange-is-not-validation
An @api.onchange method that RAISES, or returns a warning while taking the
value back -- it is trying to enforce an invariant the web client is the
only thing that will apply.
The regex this replaces read 400 characters after the decorator and could
not see where the method ended. All three of its false positives are the
same failure: a four-line onchange that assigns one field, followed by an
action method that raises. `_onchange_variable_id` sets a target model and
the UserError 200 characters below it belongs to `action_remap`. The
window crossed the function boundary, and a regex has no way not to.
Scoped to the decorated function, that shape is silent, and the four true
positives -- an onchange that really does raise ValidationError, and ones
that return the warning dict -- all stand.
A constraint in the file clears it when it covers a field the onchange
watches: an onchange beside a real @api.constrains on the same field is
the faster feedback the rule explicitly allows. Until 2026-09-27 ANY
constraint anywhere in the file cleared it, so a UNIQUE(reference)
silenced an onchange that raised on credit_limit. _sql_constraints counts
on 16 through 18 and on an unknown series, because it IS a constraint
there, and not when the tree pins 19 or later, where the registry never
creates it (_constrained_fields).
Narrowing the guard alone added 36 findings and 11 read true, because the
wide guard had been hiding a second problem: a returned warning dict was
read as a refusal whatever it said. Most were notices -- "this reference
already exists", "the rate is far from the previous one" -- that the user
saves past in the form as easily as in an import, so nothing is enforced
anywhere. A warning counts now only when its branch also takes the value
back (_refuses_in_branch). Both together: OCA unchanged, OpenSPP +4 (3
true), core +11 (8 true) and -17, all 17 notices. ARCHITECTURE 3.67, 3.68.
A check the server repeats in the same class also backs it: the same
raised sentence, or the same check helper handed the watched field, in
create(), write(), an inverse or an @api.constrains method
(_server_repeats_the_check). Core 2,339 -> 2,338, the work order's
unplanning check; nothing else moved. ARCHITECTURE 3.69.
"""
tree, _ = parse_python(code)
if tree is None:
return []
covered = _constrained_fields(
tree, _series_at_least((facts or {}).get("odoo_series"), 19.0))
findings = []
for fn in _functions(tree):
watched = _decorator_args(fn, "api", "onchange")
if watched is None:
continue
if covered is EVERY_FIELD or (watched is UNREADABLE and covered):
continue
if watched is not UNREADABLE and any(w.split(".")[0] in covered
for w in watched):
continue
if _server_repeats_the_check(tree, fn, watched):
continue
# Anchored on the decorator, not on the def below it. That is where
# the rule is, where a reader looking the finding up will look, and
# where the marker goes when somebody has already decided about it --
# a FunctionDef's own lineno is the `def`, one line past all three.
at = fn.lineno
for dec in fn.decorator_list:
func = dec.func if isinstance(dec, ast.Call) else dec
if isinstance(func, ast.Attribute) and func.attr == "onchange":
at = dec.lineno
break
for node in ast.walk(fn):
spot = None
if isinstance(node, ast.Raise):
exc = node.exc
name = _final_name(exc.func) if isinstance(exc, ast.Call) \
else _final_name(exc)
if name and ("Error" in name or "Warning" in name):
spot = node.lineno
elif isinstance(node, ast.Dict) and any(
isinstance(key, ast.Constant) and key.value == "warning"
for key in node.keys) and _refuses_in_branch(fn, node, watched):
spot = node.lineno
if spot is None:
continue
findings.append((
at,
"@api.onchange '{0}' refuses a value, and onchange runs in "
"the web client form only -- an import, an RPC call, a cron "
"or a direct create() never triggers it, so the record enters "
"the database unchallenged".format(fn.name),
None))
break
return findings
# A query decides WHICH records you get to see, and dropping ir.rule widens
# that set. Every one of the 6 hand-classified true positives is one of these.
SUDO_QUERIES = ("search", "search_count", "search_read", "read_group")
# These act on a recordset the caller already holds. Dropping ir.rule still
# matters -- it governs write and read permission as well as visibility -- but
# it widens what you may DO with a record rather than which records exist for
# you, and nothing measured here has yet found a real one.
SUDO_TOUCHES = ("create", "write", "unlink", "copy", "read")
# The grade the write and escape clauses carry, per finding rather than per
# rule. `unproven` is the corpus's word for a real defect nothing measured
# here has met yet, and that is exactly their position: the read clause has
# 6 hand-classified true positives across two trees, and these two have 0 in
# 261,000 lines. Every one of the 23 they found was read, and every one was
# an administrator by construction -- a demo-data seeder, an ACL-gated
# accounting wizard -- or a framework model with no tenant boundary at all.
#
# Grading them rather than deleting them is the same call §3.46 made about
# decimal-for-money: a clause the rule's own sentence asks for, measured at
# zero, is reported and not trusted to stop a build. It costs the read
# clause nothing, because a grade travels with the finding and not the rule.
UNPROVEN = "unproven"
def _escalates(call):
"""True when this `.sudo(...)` call turns record rules OFF.
`sudo(False)` turns them back ON. It is how a method that must run under
the caller's own rights says so, and odoo core does it 60 times --
`self = self.sudo(False)` in mail's `_check_attachments_access`, whose
own 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 fix as the defect, which is worse
than missing the defect: it is the one kind of finding that teaches a
reader the checker cannot be trusted. An argument that is not a literal
is unreadable here and counts as escalation, because `sudo(flag)` with a
computed flag can be either and the finding should stand.
"""
if not (isinstance(call, ast.Call) and isinstance(call.func, ast.Attribute)
and call.func.attr == "sudo"):
return False
given = list(call.args) + [k.value for k in call.keywords if k.arg == "flag"]
for arg in given:
if isinstance(arg, ast.Constant) and not arg.value:
return False
# `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. odoo core
# writes it four times and once with the comment "Unsudo the invoice
# after creation if not already sudoed". `sudo(self.env.su or
# field.compute_sudo)` is NOT this: the `or` can turn it on, so only
# the bare attribute is read as ambient.
if isinstance(arg, ast.Attribute) and arg.attr == "su":
return False
return True
def _under_sudo(node):
"""The receiver of `<x>.sudo().<method>(...)`, or None."""
if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)):
return None
inner = node.func.value
if isinstance(inner, ast.Call) and _escalates(inner):
return inner.func.value
return None
# Framework models odoo core gives no record rule, so sudo() on them has nothing
# to drop. Measured over every ir.rule record in odoo core's XML: the only ir.*
# models with one are ir.default, ir.embedded.actions, ir.filters, ir.ui.view and
# ir.ui.view.custom. These four are the registry: no company, no owner, no
# tenant. They are named rather than derived because a tree under review rarely
# contains base, so whether base rules them is otherwise UNKNOWN -- which is how
# three sudo() searches on ir.model stayed live on OCA after the AST move.
#
# ir.attachment is deliberately NOT here. It has no record rule either, but its
# own _search filters by the caller's access to res_model unless the env is su,
# so sudo() on it drops a real access check under another name.
RULELESS_REGISTRY_MODELS = frozenset([
"ir.model", "ir.model.fields", "ir.model.data", "ir.module.module",
])
# A domain leaf ties a search to held records only through these. `!=`, `<`
# and `not in` select everything else, which is the opposite of scoping.
_HELD_OPERATORS = frozenset(["=", "in", "child_of"])
def _chain_escalates(node):
"""Does any call along this chain turn record rules off?"""
while isinstance(node, (ast.Attribute, ast.Subscript, ast.Call)):
if isinstance(node, ast.Call) and _escalates(node):
return True
node = node.func if isinstance(node, ast.Call) else node.value
return False
def _chain_calls(node, name):
while isinstance(node, (ast.Attribute, ast.Subscript, ast.Call)):
if isinstance(node, ast.Call) and _final_name(node.func) == name:
return True
node = node.func if isinstance(node, ast.Call) else node.value
return False
# Calls that hand back records the caller could already see, or a subset of
# them. Anything else on a `self` chain is a helper whose body is not read here,
# and it may sudo inside: a controller's `self._get_search_countries()` rooted at
# `self` said nothing about whose rules produced the countries.
_RULE_PRESERVING_CALLS = frozenset([
"filtered", "filtered_domain", "mapped", "sorted", "with_context",
"with_company", "exists", "search", "grouped",
])
def _only_rule_preserving_calls(node):
while isinstance(node, (ast.Attribute, ast.Subscript, ast.Call)):
if isinstance(node, ast.Call) and _final_name(node.func) not in _RULE_PRESERVING_CALLS:
return False
node = node.func if isinstance(node, ast.Call) else node.value
return True
def _held_recordset(node, fn, depth=0):
"""Is this a recordset the caller obtained under its own record rules?
`self` and anything reached from it without sudo: `self.filtered(...)`,
`self.journal_id`, `self.env.user.partner_id`, a plain `search`. A name
counts when every assignment to it does, and a loop variable when what it
loops over does. `browse(ids)` does not count: browse applies no rule, so
it holds whatever ids it was handed, and `env.ref(...)` does not either,
for the same reason. Anything unreadable is not held, because every True
here suppresses a finding.
"""
if node is None or depth > 4 or _chain_escalates(node) or _chain_calls(node, "browse") or _chain_calls(node, "ref"):
return False
root = _chain_root(node)
if root == "self":
return _only_rule_preserving_calls(node)
if root == "request":
# The user's own records and company, never an arbitrary lookup.
text = ast.unparse(node)
return text.startswith(("request.env.user", "request.env.company"))
if root is None or fn is None:
return False
values = _assigned_values(fn, root)
if values:
return all(_held_recordset(v, fn, depth + 1) for v in values)
for loop in ast.walk(fn):
if isinstance(loop, (ast.For, ast.comprehension)) \
and isinstance(loop.target, ast.Name) and loop.target.id == root:
return _held_recordset(loop.iter, fn, depth + 1)
return False
def _held_ids(node, fn, depth=0):
"""Ids of held records: `x.id`, `x.ids`, a list of them, or a name bound to one."""
if depth > 4:
return False
if isinstance(node, (ast.List, ast.Tuple)):
real = [e for e in node.elts
if not (isinstance(e, ast.Constant) and e.value in (False, None))]
return bool(real) and all(_held_ids(e, fn, depth + 1) for e in real)
if isinstance(node, ast.Attribute) and node.attr in ("id", "ids"):
return _held_recordset(node.value, fn, depth + 1)
if isinstance(node, ast.Name) and fn is not None:
values = _assigned_values(fn, node.id)
return bool(values) and all(_held_ids(v, fn, depth + 1) for v in values)
return False
def _tied_to_held_records(call, fn):
"""Does a domain leaf in this call's arguments pin it to held records?
`('employee_id', 'in', self.ids)` returns the contracts of employees the
caller can already see, and `('journal_id', '=', self.journal_id.id)` the
moves of one journal, which belongs to one company. That is the rule's
advice followed in a spelling the rule-field test cannot read: 18 of the
44 findings the global-rule test added on core (ARCHITECTURE 3.60, 3.64).
A name passed as the domain is followed to what it was assigned.
"""
exprs = []
for arg in list(call.args) + [k.value for k in call.keywords]:
exprs.append(arg)
if isinstance(arg, ast.Name):
exprs.extend(_assigned_values(fn, arg.id) or ())
for expr in exprs:
for leaf in ast.walk(expr):
if isinstance(leaf, (ast.Tuple, ast.List)) and len(leaf.elts) == 3 \
and isinstance(leaf.elts[0], ast.Constant) \
and isinstance(leaf.elts[0].value, str) \
and isinstance(leaf.elts[1], ast.Constant) \
and leaf.elts[1].value in _HELD_OPERATORS \
and _held_ids(leaf.elts[2], fn):
return True
return False
# Prefix operators of an Odoo domain and how many terms each one takes.
_DOMAIN_ARITY = {"|": 2, "&": 2, "!": 1}
def _anded_leaves(elts):
"""Leaves ANDed at the top level of a domain list, or None when the list
is not a complete domain.
A domain is prefix notation: `'|', a, b` is a OR b, and the terms left
over at the top are ANDed. Only a leaf reached through `&` alone is
guaranteed to hold for every record returned; one under `|` or `!` is not.
A list with an operator short of terms, `['|'] + domain`, is a fragment
whose meaning depends on what it is glued to, so it answers None.
"""
def term(i):
if i >= len(elts):
return None, []
op = elts[i]
if isinstance(op, ast.Starred):
return None, [] # a spliced list could be a fragment
if isinstance(op, ast.Constant) and op.value in _DOMAIN_ARITY:
j, kept = i + 1, []
for _ in range(_DOMAIN_ARITY[op.value]):
j, inner = term(j)
if j is None:
return None, []
kept.extend(inner)
return j, kept if op.value == "&" else []
return i + 1, [op]
i, leaves = 0, []
while i < len(elts):
i, inner = term(i)
if i is None:
return None
leaves.extend(inner)
return leaves
def _is_published_leaf(node):
return isinstance(node, (ast.Tuple, ast.List)) and len(node.elts) == 3 \
and all(isinstance(e, ast.Constant) for e in node.elts) \
and [e.value for e in node.elts] == ["website_published", "=", True]
def _domain_keeps_published(expr, fn, depth=0):
"""Does every record this domain can return have website_published = True?
Followed through the spellings a controller builds a domain in: a list
literal, `a + b`, `list(a)`, `a.copy()`, and a name every plain assignment
to which qualifies. `domain += [...]` and `domain.append(leaf)` only AND
more terms on, so they keep it, unless what they add is a fragment; an
`insert` can put an operator in front of the leaf, so it does not.
Anything unreadable answers False, because every True suppresses a finding.
"""
if depth > 4:
return False
if isinstance(expr, (ast.List, ast.Tuple)):
leaves = _anded_leaves(expr.elts)
return leaves is not None and any(_is_published_leaf(x) for x in leaves)
if isinstance(expr, ast.BinOp) and isinstance(expr.op, ast.Add):
sides = (expr.left, expr.right)
if any(isinstance(s, (ast.List, ast.Tuple)) and _anded_leaves(s.elts) is None
for s in sides):
return False
return any(_domain_keeps_published(s, fn, depth + 1) for s in sides)
if isinstance(expr, ast.Call):
if isinstance(expr.func, ast.Name) and expr.func.id == "list" \
and len(expr.args) == 1:
return _domain_keeps_published(expr.args[0], fn, depth + 1)
if isinstance(expr.func, ast.Attribute) and expr.func.attr == "copy" \
and not expr.args:
return _domain_keeps_published(expr.func.value, fn, depth + 1)
return False
if not isinstance(expr, ast.Name) or fn is None \
or _assigned_values(fn, expr.id) is None:
return False
plain = []
for node in ast.walk(fn):
if isinstance(node, ast.Assign) and any(
isinstance(t, ast.Name) and t.id == expr.id for t in node.targets):
plain.append(node.value)
elif isinstance(node, ast.AugAssign) and isinstance(node.target, ast.Name) \
and node.target.id == expr.id:
added = node.value
if not isinstance(node.op, ast.Add) or (
isinstance(added, (ast.List, ast.Tuple))
and _anded_leaves(added.elts) is None):
return False
elif isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and isinstance(node.func.value, ast.Name) \
and node.func.value.id == expr.id:
if node.func.attr == "insert":
return False
if node.func.attr in ("append", "extend") and any(
isinstance(a, ast.Constant) for a in node.args):
return False
return bool(plain) and all(_domain_keeps_published(v, fn, depth + 1)
for v in plain)
def _public_website_route(fn):
"""Is this function an @http.route(..., auth='public', website=True)?"""
if not isinstance(fn, (ast.FunctionDef, ast.AsyncFunctionDef)):
return False
for dec in fn.decorator_list:
if not isinstance(dec, ast.Call) or _final_name(dec.func) != "route":
continue
kw = {k.arg: k.value for k in dec.keywords if k.arg}
auth, website = kw.get("auth"), kw.get("website")
if isinstance(auth, ast.Constant) and auth.value == "public" \
and isinstance(website, ast.Constant) and website.value is True:
return True
return False
def _published_on_a_public_page(call, fn):
"""Is this a public website page reading only what it publishes?
website_customer's /customers and website_crm_partner_assign's /partners
list, group and count published partners under sudo(), because the
anonymous visitor's own rules would hide most of them. Publishing is the
record owner's decision to show it to everybody, and on a model with
website.published.multi.mixin, searching website_published = True inside a
website request also applies the current website's domain
(_search_website_published, website/models/mixins.py). 12 of the 44 false
positives the global-rule test added on core were this (ARCHITECTURE 3.60,
3.65).
Narrow on purpose: a read, the literal leaf ('website_published', '=',
True) ANDed at the top of the domain, inside a route that is both public
and a website page. A published flag compared to anything but True, an
`is_published` leaf, which applies no website, or the same search in a
model method, stays a finding: website_sale's set_sequence_up is that.
"""
if call.func.attr not in SUDO_QUERIES or not _public_website_route(fn):
return False
domain = call.args[0] if call.args else next(
(k.value for k in call.keywords if k.arg == "domain"), None)
return domain is not None and _domain_keeps_published(domain, fn)
def sudo_drops_record_rules(code, facts=None):
"""odoo.sudo-bypasses-record-rules
Three clauses, because the rule has three: scope sudo to the narrowest
operation, re-apply company filtering by hand, and never pass a sudo
recordset onward.
1. a read under sudo -- search, search_read, read_group, read
2. a write under sudo -- create, write, unlink, copy
3. a sudo recordset bound to a name and then handed to something else
Only the first was checked. The `--harvest` clustering says what that
cost: `.sudo(` is the largest miss cluster in the corpus's recall
measurement at 70 missed against 6 caught, and 47 of those 70 are files
where a checker fired somewhere else -- a rule aimed wrong rather than a
rule that is missing. Counted on the OpenSPP tree, the clause that was
checked has 116 call sites and the clause that was not has 136, so the
unchecked half was the larger one. One of the misses the README names,
*"remove sudo() when importing record"*, is a create the search-only
checker could not see at all.
ir.rule governs read, write, create and unlink alike, so the same
record-rule trigger gates every clause: where the tree declares no rule
that filters this model, sudo drops nothing and there is no finding. The
trigger only ever SUPPRESSES. A model the walk could not name, a tree that
was not walked, a partial walk -- each reads UNKNOWN and the finding
stands, because "no rule was found" and "nobody looked" must not be the
same answer on a critical rule. `nexa check` on one file therefore behaves
exactly as it did before triggers existed.
Clause 3 fires at the binding, not at the use. `partners =
self.env['res.partner'].sudo()` is a recordset carrying an environment
with su=True, and every call made on it downstream inherits that -- which
is the whole reason the rule's sentence says not to pass one on. Reported
where the escape happens, once, rather than at each later read.
What it over-reported, measured: 14 of the 20 surviving read findings
constrained the search to records the caller had already identified -- a
group id, a partner id, an area -- which is the rule's advice followed in a
spelling the rule did not recognise. Three predicates now read that
without evaluating the domain: the model's own rule fields (see rescoped),
a leaf tying the search to ids of records the caller holds under its own
rules (_tied_to_held_records, ARCHITECTURE 3.64), and a public website
page reading only published records (_published_on_a_public_page, 3.65).
None touches the six OpenSPP true positives, which are unscoped sweeps.
"""
tree, _ = parse_python(code)
if tree is None:
return []
classes = [n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)]
constants = _module_string_constants(tree)
def rescoped(node, model):
# Did this call re-apply the scoping the rule it bypasses is about?
#
# The old test was the literal word `company_id`, from the rule's own
# correct sample. That is one spelling of one kind of rule, and the
# docstring recorded what it cost: 14 of 20 surviving read findings
# were scoped by a group, a partner or an area instead, and read as
# unscoped. Every OpenSPP case model is governed by `case_worker_id`
# or `case_id.case_worker_id` and carries no company rule at all.
#
# So the call is now compared against the fields the model's OWN
# ir.rule filters on, which is not a guess about the domain -- it is
# the domain, read off the tree that declares it. A create naming
# `case_id` lands inside the caller's visibility because that is the
# link the rule follows. company_id stays as the fallback for a model
# whose rules this could not read, so nothing that was exempt before
# stops being exempt now.
# The ARGUMENTS only. Unparsing the whole call includes the receiver,
# and the model name is part of it: `spp.group.membership` contains
# the word `group`, which is one of that model's own rule fields, so
# reading the whole chain exempted an unscoped sweep of every
# membership in the database -- a finding classified true by hand.
#
# An argument that is a bare name is followed to what it was assigned,
# because `create(vals)` otherwise hides every field it sets behind
# four letters. The trace is the same local one the other checkers
# use, and a name it cannot resolve contributes nothing.
pieces = []
for arg in list(node.args) + [k.value for k in node.keywords]:
try:
pieces.append(ast.unparse(arg))
except Exception: # pragma: no cover
continue
if isinstance(arg, ast.Name):
for value in _assigned_values(owner.get(id(node)), arg.id) or ():
try:
pieces.append(ast.unparse(value))
except Exception: # pragma: no cover
pass
call_src = " ".join(pieces)
if _tied_to_held_records(node, owner.get(id(node))) \
or _published_on_a_public_page(node, owner.get(id(node))):
return True
def named(name):
return re.search(r"\b" + re.escape(name.split(".", 1)[0]) + r"\b",
call_src) is not None
# Every GLOBAL rule, not any rule. Odoo ANDs the rules that carry no
# groups for everybody and ORs group rules per audience, so naming
# one field of one rule re-applies that rule and no other.
# website_sale's set_sequence_up names website_published, the field of
# product.template's portal and public rule, and was exempt while the
# global company rule was dropped. Where the model has no global rule
# the old test stands: any rule's field, then company_id.
required = [fields for is_global, fields in
(context.model_rule_sets(facts, model) if model else ())
if is_global and fields]
if required:
return all(any(named(name) for name in fields) for fields in required)
fields = context.model_rule_fields(facts, model) if model else ()
for name in fields or ("company_id",):
if named(name):
return True
return "company_id" in call_src
owner = _function_of(tree)
def model_of(receiver):
# `self.env['x'].sudo()` names its model in the chain. `Plan.sudo()`
# does not, and every one of the write clause's first findings on
# OpenSPP's demo generators was that shape -- `Plan =
# self.env['spp.case.plan']` a few lines up, then fifteen writes on
# the name. Unresolved, the trigger reads UNKNOWN and cannot suppress;
# followed through the assignment, it can answer. Every assignment to
# the name must agree, or the answer is still None.
model = _env_model_of(receiver)
if model is not None or not isinstance(receiver, ast.Name):
return model
if receiver.id == "self":
# `self` is the model the enclosing class defines or extends, when
# one name says which: tracking_manager's `self.sudo().search()`
# inside `_inherit = "ir.model"` is a search on ir.model.
around = [c for c in classes
if c.lineno <= receiver.lineno <= getattr(c, "end_lineno", c.lineno)]
if around:
return _own_model_name(around[-1], constants)
values = _assigned_values(owner.get(id(receiver)), receiver.id)
if not values:
values = [v for n in ast.walk(tree) if isinstance(n, ast.Assign)
for t in n.targets if isinstance(t, ast.Name)
and t.id == receiver.id for v in (n.value,)]
found = {_env_model_of(v) for v in values}
return found.pop() if len(found) == 1 and None not in found else None
def ruled_out(receiver):
model = model_of(receiver)
if model in RULELESS_REGISTRY_MODELS:
return True, model
return context.model_is_record_ruled(facts, model) == context.ABSENT, model
findings, seen = [], set()
def report(line, text, grade=None):
if line not in seen:
seen.add(line)
findings.append((line, text, grade))
for node in ast.walk(tree):
receiver = _under_sudo(node)
if receiver is None:
continue
method = node.func.attr
if method not in SUDO_QUERIES and method not in SUDO_TOUCHES:
continue
absent, model = ruled_out(receiver)
if absent:
continue
if rescoped(node, model):
continue
where = " on {0}".format(model) if model else ""
if method in SUDO_QUERIES:
report(node.lineno,
"sudo().{0}(){1} runs with record rules off -- ir.rule "
"filtering, including multi-company isolation, does not "
"apply and the result looks correct in single-company "
"testing".format(method, where))
else:
report(node.lineno,
"sudo().{0}(){1} acts with record rules off -- ir.rule "
"governs write, create, unlink and read as well as "
"visibility, so this reaches a record the caller's own "
"rules would not let it touch".format(method, where),
UNPROVEN)
# Clause 3: a sudo recordset given a name, then handed on.
#
# The bare name only, as an argument or as the whole returned value.
# `return partners.mapped('name')` hands back a list of strings and the
# elevated environment does not travel with it, while
# `return partners.filtered(...)` does -- telling those apart means
# knowing which recordset methods return recordsets, so neither is
# reported. Clause 3 under-reports by construction, which is the right
# direction for a new clause on a critical rule: the search clause's
# first version fired 106 times for 6 real ones, and a flood is how a
# checker gets switched off.
for fn in _functions(tree):
escapes = set()
for node in ast.walk(fn):
if isinstance(node, ast.Call):
if _under_sudo(node) is None:
escapes.update(a.id for a in node.args
if isinstance(a, ast.Name))
escapes.update(k.value.id for k in node.keywords
if isinstance(k.value, ast.Name))
elif isinstance(node, ast.Return) and isinstance(node.value, ast.Name):
escapes.add(node.value.id)
for node in ast.walk(fn):
if not isinstance(node, ast.Assign) or not isinstance(node.value, ast.Call):
continue
call = node.value
if not _escalates(call):
continue
bound = [t for t in _targets(node)]
# `self = self.sudo()` rebinds the method's own receiver, and
# every ORM call below it in the method inherits the elevated
# environment without naming sudo anywhere. It is the escape
# clause at its widest -- the recordset is not passed on, it
# replaces the thing the whole method is written against.
#
# This is the miss the README names by hand. OCA server-tools
# cbefaa511485, "[FIX] excel_import_export: remove sudo() when
# importing record", deletes exactly this line 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."
#
# Three sites in 260,000 lines, so it is narrow as well as real.
# The same commit ADDS `self.sudo().env["ir.model.data"]`, a
# deliberate and narrow elevation, which is the evidence for not
# also flagging that shape.
rebind = isinstance(call.func.value, ast.Name) \
and call.func.value.id in bound
if rebind:
report(node.lineno,
"'{0}' is rebound to its own sudo form, so every ORM "
"call below this line in the method runs with record "
"rules off -- including ones that name no sudo at all"
.format(call.func.value.id))
continue
if not any(name in escapes for name in bound):
continue
absent, model = ruled_out(call.func.value)
if absent:
continue
where = " of {0}".format(model) if model else ""
report(node.lineno,
"this sudo recordset{0} is passed on -- the elevated "
"environment travels with it, so every read and write made "
"on it downstream also runs with record rules off"
.format(where), UNPROVEN)
return sorted(findings)
def _env_model_of(node):
"""The model name at the head of a chained call, or None.
Resolves `self.env["res.partner"]` and `env["res.partner"]` through any
number of intervening attributes. A head that is a plain name -- the
`Partner` in `Partner.sudo().search(...)` -- is not resolvable without
following the assignment, so it returns None and the caller treats the
record-rule question as unanswered rather than answered no.
"""
while isinstance(node, (ast.Call, ast.Attribute)):
node = node.func if isinstance(node, ast.Call) else node.value
if isinstance(node, ast.Subscript) and isinstance(node.slice, ast.Constant) \
and isinstance(node.slice.value, str):
target = node.value
name = target.attr if isinstance(target, ast.Attribute) else (
target.id if isinstance(target, ast.Name) else None)
if name == "env":
return node.slice.value
return None
CHECKS = {
"handler_order_shadows_a_subclass": handler_order_shadows_a_subclass,
"sudo_drops_record_rules": sudo_drops_record_rules,
"float_equality_against_zero": float_equality_against_zero,
"path_join_takes_an_external_name": path_join_takes_an_external_name,
"float_coercion_of_external_money": float_coercion_of_external_money,
"onchange_tries_to_enforce": onchange_tries_to_enforce,
"check_company_missing_on_company_owned_relation":
check_company_missing_on_company_owned_relation,
"commit_inside_a_loop": commit_inside_a_loop,
"depends_names_unread_fields": depends_names_unread_fields,
"sql_built_by_string_formatting": sql_built_by_string_formatting,
"constrains_lists_every_field_read": constrains_lists_every_field_read,
"constrains_declares_a_dotted_name": constrains_declares_a_dotted_name,
"depends_covers_compute_reads": depends_covers_compute_reads,
"ondelete_leaves_a_dangling_read": ondelete_leaves_a_dangling_read,
"prefer_sql_constraint_applies": prefer_sql_constraint_applies,
"parent_can_form_a_cycle": parent_can_form_a_cycle,
"editable_record_without_noupdate": editable_record_without_noupdate,
"naive_time_leaves_or_dates_a_day": naive_time_leaves_or_dates_a_day,
"legacy_sql_constraints_on_19": legacy_sql_constraints_on_19,
}
# Checkers that read whole-tree facts as well as the file in front of them. A
# set rather than signature introspection: which checkers are conditional on the
# tree is a design fact worth being able to read off one line, and a checker that
# grew a parameter by accident would otherwise start receiving one silently.
CONTEXT_CHECKS = frozenset([
"ondelete_leaves_a_dangling_read",
"check_company_missing_on_company_owned_relation",
# Reads models_record_ruled: whether sudo drops anything depends on whether
# the tree declares a rule that filters.
"sudo_drops_record_rules",
# Reads odoo_series: whether a _sql_constraints block is a constraint at
# all depends on which Odoo is going to load it.
"prefer_sql_constraint_applies",
# Reads odoo_series, and whether a tree was walked at all: whether a
# _sql_constraints block exists in the database depends on the series.
"legacy_sql_constraints_on_19",
# Reads odoo_series: a _sql_constraints block backs an onchange only on a
# series that creates it.
"onchange_tries_to_enforce",
# Reads the ORM facts: whether a compute's field is stored, which
# @api.depends Odoo merges into the method, and what a declared computed
# field depends on in turn. A gap checker as well.
"depends_covers_compute_reads",
])
# Checkers that can meet input they cannot decide about -- a decorator whose
# arguments are computed -- and say so instead of guessing. Same reasoning as
# CONTEXT_CHECKS: which checkers have a second channel is a design fact, and one
# that grew a parameter by accident should not start receiving one silently.
GAP_CHECKS = frozenset([
"constrains_lists_every_field_read",
"constrains_declares_a_dotted_name",
"depends_covers_compute_reads",
])
def run_check(name, code, facts=None, gaps=None):
"""Findings from one named checker. Unknown name raises, never passes.
A renamed checker that silently returned 'clean' would turn a critical rule
into a permanent false pass, which is the exact failure the corpus exists to
prevent. corpus.py validate calls this for every rule carrying a
semantic_check, so a typo fails the build.
`gaps` collects what a checker could not decide -- a method whose decorator
is computed rather than written out. Not findings: nothing is wrong with
@api.constrains(*FIELDS), it is simply outside a static pass. None means the
caller is not collecting them.
`facts` is context.tree_facts for the tree this file came from, or None. The
checkers in CONTEXT_CHECKS read a trigger off it; None means no tree was
walked and every trigger reads UNKNOWN, which leaves those checkers behaving
exactly as they did before triggers existed.
Returns (line, message, grade) throughout. `grade` is a likelihood the
finding earned on its own evidence -- a trigger that fired -- or None to keep
the grade its rule already carries. Normalised here so the six checkers with
no opinion about it do not each have to return a third element saying so.
"""
if name not in CHECKS:
raise KeyError("no semantic checker named '{0}'".format(name))
if name in CONTEXT_CHECKS and name in GAP_CHECKS:
found = CHECKS[name](code, facts, gaps=gaps)
elif name in CONTEXT_CHECKS:
found = CHECKS[name](code, facts)
elif name in GAP_CHECKS:
found = CHECKS[name](code, gaps=gaps)
else:
found = CHECKS[name](code)
return [hit if len(hit) == 3 else (hit[0], hit[1], None) for hit in found]
# ------------------------------------------------- imports and symbol checks
# Symbols a generated Odoo or Python snippet uses constantly and cannot use
# without an import. Curated rather than derived: a general undefined-name pass
# over a method-shaped fragment flags the surrounding class it was lifted from
# and buries the real finding under it.
NEEDS_IMPORT = {
"ValidationError": "odoo.exceptions",
"UserError": "odoo.exceptions",
"AccessError": "odoo.exceptions",
"MissingError": "odoo.exceptions",
"api": "odoo",
"fields": "odoo",
"models": "odoo",
"float_compare": "odoo.tools",
"float_is_zero": "odoo.tools",
"float_round": "odoo.tools",
"timedelta": "datetime",
"Decimal": "decimal",
"relativedelta": "dateutil.relativedelta",
}
def imported_names(tree):
"""Every name the module binds by importing it, aliases resolved."""
names = set()
for node in ast.walk(tree):
if isinstance(node, ast.Import):
for alias in node.names:
names.add(alias.asname or alias.name.split(".")[0])
elif isinstance(node, ast.ImportFrom):
for alias in node.names:
names.add(alias.asname or alias.name)
return names
def imported_modules(tree):
"""Every module a tree reaches for, by its real name, aliases seen through.
Deliberately not imported_names(). That one answers "what does this module
BIND", which is what resolving an undefined name needs. An allowlist needs
"what does this REACH FOR", and the two disagree in both directions:
from decimal import Decimal binds Decimal, reaches decimal
import os as math binds math, reaches os
A relative import resolves to a package this snippet is not part of, so it
is reported as "." and can never match an allowlist entry.
"""
modules = set()
for node in ast.walk(tree):
if isinstance(node, ast.Import):
for alias in node.names:
modules.add(alias.name.split(".")[0])
elif isinstance(node, ast.ImportFrom):
if node.level:
modules.add(".")
elif node.module:
modules.add(node.module.split(".")[0])
return modules
def bound_names(tree):
"""Names bound by assignment, def, class, except, or a for/with target.
A snippet that defines its own 'fields' helper is not missing an import.
"""
names = set()
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
names.add(node.name)
elif isinstance(node, ast.Name) and isinstance(node.ctx, ast.Store):
names.add(node.id)
elif isinstance(node, ast.ExceptHandler) and node.name:
names.add(node.name)
elif isinstance(node, ast.arg):
names.add(node.arg)
return names
def is_lifted_fragment(tree, notes):
"""True when the block is a method shown without the class it belongs to.
WHY THIS GATES THE IMPORT CHECK: a fragment like that was never going to
carry an import header. The imports live in the module the answer quoted
from, which was not part of the answer. Reporting them as missing is the
same category of noise that the undefined-name analysis already declines to
report -- and it is worse than useless, because a medium finding is enough
to turn a correct answer's verdict into WARN and bury whatever else the run
found.
Two signals, either one sufficient: parse_fragment had to repair the block
to make it parse at all (notes), or a function takes self or cls, which no
module-level function does.
"""
if notes:
return True
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
args = list(node.args.posonlyargs) + list(node.args.args)
if args and args[0].arg in ("self", "cls"):
return True
return False
def missing_imports(tree):
"""Curated symbols used without an import, as (line, message).
Catches the failure the prompt explicitly asks the model to avoid -- 'Code
first: runnable, imports included' -- and which a 7B drops routinely once
the answer is long enough for that instruction to fall out of attention.
"""
have = imported_names(tree) | bound_names(tree)
findings, seen = [], set()
for node in ast.walk(tree):
if not isinstance(node, ast.Name) or not isinstance(node.ctx, ast.Load):
continue
if node.id in have or node.id in seen or node.id not in NEEDS_IMPORT:
continue
seen.add(node.id)
findings.append((node.lineno,
"'{0}' is used but never imported (expected: from {1} import {0})"
.format(node.id, NEEDS_IMPORT[node.id])))
return findings
def unresolvable_imports(tree):
"""Imported modules that are not installed here, as (line, message).
Implementation verification rather than plausibility: find_spec asks the
interpreter whether the module actually exists on this machine, which is a
different and stronger question than whether the import line looks
reasonable.
odoo is skipped. It is absent from this environment by design -- Nexa runs
beside Odoo projects, not inside one -- so reporting it would be noise on
every single Odoo answer.
"""
findings = []
for node in ast.walk(tree):
if isinstance(node, ast.Import):
modules = [a.name for a in node.names]
elif isinstance(node, ast.ImportFrom):
modules = [node.module] if node.module and not node.level else []
else:
continue
for module in modules:
root = module.split(".")[0]
if root in ("odoo", "openerp"):
continue
try:
found = importlib.util.find_spec(root) is not None
except (ImportError, ValueError, ModuleNotFoundError):
found = False
if not found:
findings.append((node.lineno,
"module '{0}' is not installed here, so the import "
"cannot be verified".format(root)))
return findings
# ------------------------------------------------------- comment stripping
def _docstring_starts(code):
"""Character offsets where a docstring literal opens.
A docstring is prose, and a regex cannot tell prose from code -- which is
the whole reason comments are stripped before matching. String literals
were left standing, and classifying 50 findings by hand found the cost:
two of them were `eval()` named in a docstring in a sentence arguing
AGAINST using it, and a `datetime.now()` inside a doctest example. Both
read as live code.
Only docstrings, not every string. A triple-quoted SQL query is data the
checkers have a legitimate interest in, and `_SQL_COLUMNS` reads one. The
statement-level bare string an AST calls a docstring is the one place the
file is guaranteed to be talking ABOUT code rather than in it.
Offsets, not spans: the scanner already knows how to find a literal's end,
and asking it to do that is one way of measuring the string rather than
two. `col_offset` is a byte offset, which parts company with a character
offset on a line holding non-ASCII -- so the result is checked against the
quote it claims to point at and dropped when it does not, leaving that
docstring intact rather than blanking the wrong run of characters.
"""
tree, _ = parse_python(code)
if tree is None:
return frozenset()
line_start, offset = [], 0
for row in code.splitlines(True):
line_start.append(offset)
offset += len(row)
starts = set()
holders = (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)
for node in ast.walk(tree):
if not isinstance(node, holders):
continue
body = getattr(node, "body", None)
if not body:
continue
first = body[0]
if not (isinstance(first, ast.Expr) and isinstance(first.value, ast.Constant)
and isinstance(first.value.value, str)):
continue
lineno = getattr(first.value, "lineno", 0)
if not 0 < lineno <= len(line_start):
continue
at = line_start[lineno - 1] + first.value.col_offset
# The guard the docstring above describes. A prefix of anything but
# indentation is possible -- `def f(): "doc"` -- but it is ASCII in
# every spelling that reaches here, so a miss means the offset is
# wrong and the safe answer is to leave the literal alone.
if at < len(code) and code[at] in ("'", '"'):
starts.add(at)
return frozenset(starts)
def _blanked(text):
"""A string literal with its contents emptied, quotes and shape intact.
Same length and the same newlines, so the line AND column a finding
reports are the ones a person reading the file will see, and so a
docstring that is the whole body of a function still leaves that function
syntactically intact for anything that parses the stripped view.
"""
if len(text) < 2:
return text
quote = 3 if text[:3] in ("'''", '"""') else 1
head, body, tail = text[:quote], text[quote:-quote or None], text[-quote:]
if len(text) < quote * 2:
return text
return head + "".join("\n" if c == "\n" else " " for c in body) + tail
def strip_comments(code, kind="python"):
"""Code with its comments and docstrings removed, strings left intact.
WHY: a violation_pattern and a satisfied_pattern are regexes, and a regex
cannot tell code from prose. The adversarial counterexample probe put a
number on what that costs -- all 12 satisfied_patterns in the corpus could be
cleared by a COMMENT quoting the fix rather than applying it. Four of those
were critical rules, including odoo.onchange-is-not-validation: write the
onchange, add '# _sql_constraints = [', and the checker reported clean while
the bug shipped.
It cuts the other way too. A commented-out @api.onchange was being reported
as a live violation, which is the false positive that teaches people to
ignore a checker.
A docstring is the same argument wearing a different quote. It was left
standing here until classifying 50 findings by hand turned up two that were
prose read as code, so docstrings are now emptied and left in place -- see
_docstring_starts for why only docstrings and not every literal.
Newlines inside removed comments are preserved so that multi-line patterns
still see the line structure they were written against.
"""
out = []
i, n = 0, len(code)
triple = ("'''", '"""')
docstrings = _docstring_starts(code) if kind == "python" else frozenset()
while i < n:
ch = code[i]
if kind == "python" and code[i:i + 3] in triple:
marker = code[i:i + 3]
end = code.find(marker, i + 3)
end = n if end < 0 else end + 3
out.append(_blanked(code[i:end]) if i in docstrings else code[i:end])
i = end
continue
if ch in ("'", '"') or (kind == "js" and ch == "`"):
j = i + 1
while j < n:
if code[j] == "\\":
j += 2
continue
if code[j] == ch:
j += 1
break
if code[j] == "\n" and kind == "python":
break # unterminated single-quoted string
j += 1
out.append(_blanked(code[i:j]) if i in docstrings else code[i:j])
i = j
continue
# XML comments are stripped for both kinds: an Odoo answer mixes Python
# and view XML in the same block often enough to matter.
if code[i:i + 4] == "<!--":
end = code.find("-->", i + 4)
end = n if end < 0 else end + 3
out.append("\n" * code[i:end].count("\n"))
i = end
continue
if kind == "python" and ch == "#":
end = code.find("\n", i)
i = n if end < 0 else end
continue
if kind == "js" and code[i:i + 2] == "//":
end = code.find("\n", i)
i = n if end < 0 else end
continue
if kind == "js" and code[i:i + 2] == "/*":
end = code.find("*/", i + 2)
end = n if end < 0 else end + 2
out.append("\n" * code[i:end].count("\n"))
i = end
continue
out.append(ch)
i += 1
return "".join(out)
def looks_like_markup(code):
"""Is this a view file rather than source? Decides the comment syntax a
mutation may safely insert."""
stripped = code.lstrip()
return stripped.startswith("<") and not stripped.startswith("<<")
# ------------------------------------------------ JavaScript / TypeScript
_JS_OPEN = {"(": ")", "[": "]", "{": "}"}
_JS_CLOSE = {")": "(", "]": "[", "}": "{"}
# A slash starts a regex literal only where a value cannot already be complete.
# The standard heuristic; wrong for a few legal programs, and documented as a
# heuristic rather than sold as a parser.
_REGEX_OK_AFTER = set("(,=:[!&|?{};+-*%~^<>") | {"\n", "\r", ""}
# The other half of the same heuristic, and the half this was missing. A slash
# after one of these words is a regex too, because none of them can END a value:
# `return /re/`, `case /re/`, `typeof /re/`. Read as division instead, the regex
# BODY was scanned as code, so a character class holding one bracket -- /[(]/,
# /]/, /{/ -- was reported as an unbalanced delimiter in a file that is fine.
# Only bites when the class is unbalanced, which is why /^[^@]+@[^@]+$/ survived
# it: its brackets happen to pair.
_REGEX_OK_AFTER_WORD = frozenset([
"return", "typeof", "instanceof", "in", "of", "new", "delete", "void",
"throw", "case", "do", "else", "yield", "await",
])
def balance_js(code):
"""Delimiter balance for JS/TS, aware of strings, comments and templates.
NOT a parser. There is no JavaScript grammar in the standard library, so
this answers one question only: does every bracket that opens also close, in
the right order, outside a string or comment? That catches the truncated
answer and the run-together answer -- the two ways generated code most often
fails to be code at all -- and says nothing about anything else.
A caller wanting a real parse has to shell out to a toolchain, which this
project deliberately does not require. The layer above records that gap as
UNKNOWN instead of letting a balanced file read as a clean parse.
"""
stack, findings = [], []
i, line, n = 0, 1, len(code)
while i < n:
ch = code[i]
nxt = code[i + 1] if i + 1 < n else ""
if ch == "\n":
line += 1
i += 1
continue
if ch == "/" and nxt == "/":
while i < n and code[i] != "\n":
i += 1
continue
if ch == "/" and nxt == "*":
i += 2
while i < n - 1 and not (code[i] == "*" and code[i + 1] == "/"):
if code[i] == "\n":
line += 1
i += 1
i += 2
continue
if ch in ("'", '"'):
i, line = _skip_quoted(code, i, line, ch)
continue
if ch == "`":
i, line = _skip_template(code, i, line)
continue
if ch == "/" and _starts_a_regex(code, i):
i, line = _skip_regex(code, i, line)
continue
if ch in _JS_OPEN:
stack.append((ch, line))
elif ch in _JS_CLOSE:
if not stack:
findings.append((line, "closing '{0}' with nothing open".format(ch)))
elif stack[-1][0] != _JS_CLOSE[ch]:
opener, opened_at = stack.pop()
findings.append((line, "'{0}' closes a '{1}' opened on line {2}".format(
ch, opener, opened_at)))
else:
stack.pop()
i += 1
for opener, opened_at in stack:
findings.append((opened_at, "'{0}' opened here is never closed".format(opener)))
return findings
def _starts_a_regex(code, i):
"""Is the slash at i opening a regex literal rather than dividing?"""
token = _prev_significant(code, i)
return token in _REGEX_OK_AFTER or token in _REGEX_OK_AFTER_WORD
def _prev_significant(code, i):
"""The previous significant character -- or the whole WORD it ends.
One character cannot tell `return` from an identifier ending in n, and
that is exactly the difference between a regex and a division. Returning
the word lets the caller ask the question it actually has.
"""
j = i - 1
while j >= 0 and code[j] in " \t":
j -= 1
if j < 0:
return ""
if not (code[j].isalnum() or code[j] in "_$"):
return code[j]
end = j + 1
while j >= 0 and (code[j].isalnum() or code[j] in "_$"):
j -= 1
return code[j + 1:end]
def _skip_quoted(code, i, line, quote):
i += 1
while i < len(code):
if code[i] == "\\":
i += 2
continue
if code[i] == quote:
return i + 1, line
if code[i] == "\n":
line += 1 # unterminated; balance reports the damage after
i += 1
return i, line
def _skip_template(code, i, line):
"""Template literals nest. Backtick-dollar-brace bodies are scanned as code,
because a generated validation message routinely interpolates another
template inside itself and the braces have to balance there too."""
i += 1
while i < len(code):
if code[i] == "\\":
i += 2
continue
if code[i] == "`":
return i + 1, line
if code[i] == "\n":
line += 1
if code[i] == "$" and i + 1 < len(code) and code[i + 1] == "{":
depth = 1
i += 2
while i < len(code) and depth:
if code[i] == "{":
depth += 1
elif code[i] == "}":
depth -= 1
elif code[i] == "\n":
line += 1
elif code[i] == "`":
i, line = _skip_template(code, i, line)
continue
i += 1
continue
i += 1
return i, line
def _skip_regex(code, i, line):
i += 1
in_class = False
while i < len(code):
if code[i] == "\\":
i += 2
continue
if code[i] == "[":
in_class = True
elif code[i] == "]":
in_class = False
elif code[i] == "/" and not in_class:
return i + 1, line
elif code[i] == "\n":
return i, line + 1 # a regex cannot span lines: it was division
i += 1
return i, line
if __name__ == "__main__":
import json
import sys
if len(sys.argv) > 2 and sys.argv[1] == "check":
print(json.dumps(run_check(sys.argv[2], sys.stdin.read()), indent=2))
else:
print(__doc__)
print("checkers: " + ", ".join(sorted(CHECKS)))
|