static_analysis: flag range filters on nullable fields with no is-set guard

docs/static_analysis.md

@@ -47,6 +47,38 @@ Scans every Python file inside any `report/` directory in the app (including nes
 - Not decorated with `@frappe.whitelist()`
 - Not called directly by `execute()`
 
+### nullable_filters
+
+Scans `frappe.get_all` / `get_list` / `get_count` / `frappe.db.exists` calls for a list-form filter clause that compares a nullable `Date`, `Datetime`, `Time`, or `Data` field with a range operator (`<`, `>`, `<=`, `>=`) — cross-referenced against the DocType JSON to confirm the field is not `reqd` and has no `default`.
+
+Frappe's query builder wraps filters on nullable columns in `ifnull(col, '')`. For a comparison operator this makes `''` a valid operand — `'' < '2026-01-01'` is string-true on both MariaDB and Postgres — so a filter like `["due_date", "<", nowdate()]` silently matches rows where `due_date` is `NULL`, not just rows before the date.
+
+Warns unless the same field also has an `["field", "is", "set"]` (or `"not set"`) clause in the same filter list — Frappe's list-form filters allow two conditions on one fieldname, which is the fix:
+
+```python
+# Warns: a NULL grace_period_deadline reads as '' and '' < now is string-true,
+# so not-yet-due rows are swept up as already expired.
+frappe.get_all(
+    "Remote Cloud Bench Purchase",
+    filters=[
+        ["purchase_status", "=", "Past Due"],
+        ["grace_period_deadline", "<", now],
+    ],
+)
+
+# Clean: the "is set" guard excludes NULL rows before the comparison runs.
+frappe.get_all(
+    "Remote Cloud Bench Purchase",
+    filters=[
+        ["purchase_status", "=", "Past Due"],
+        ["grace_period_deadline", "is", "set"],
+        ["grace_period_deadline", "<", now],
+    ],
+)
+```
+
+This is always a warning, never a hard error — a caller may have already excluded NULLs earlier in the same request (e.g. via a preceding non-range filter that's mutually exclusive with NULL), and the check doesn't attempt to prove that.
+
 ### orphans
 
 Runs [Vulture](https://github.com/jendrikseipp/vulture) against the app to detect unused imports, variables, and functions. Before running, the analyzer seeds a Vulture whitelist with all discovered entry points (whitelisted functions, hooks paths, doctype controllers) so they are not incorrectly flagged.

test_utils/pre_commit/static_analysis.py

@@ -41,6 +41,7 @@ def _run(args: argparse.Namespace) -> int:
 		validate_python_calls=not args.no_python_calls,
 		validate_jinja=not args.no_jinja,
 		validate_reports=not args.no_reports,
+		validate_nullable_filters=not args.no_nullable_filters,
 		detect_orphans=not args.no_orphans,
 		min_confidence=args.min_confidence,
 	)
@@ -124,6 +125,16 @@ def _run(args: argparse.Namespace) -> int:
 		else:
 			print(f"\n[reports] {label} — OK")
 
+	# Nullable filters
+	if result.nullable_filter_result is not None:
+		nfr = result.nullable_filter_result
+		label = f"Nullable filters ({nfr.calls_checked} calls checked)"
+		if nfr.warnings:
+			any_output = True
+			_print_section(f"[nullable_filters] {label}", nfr.warnings, prefix="  WARN:  ")
+		else:
+			print(f"\n[nullable_filters] {label} — OK")
+
 	# Orphans
 	if result.orphan_result is not None:
 		orph = result.orphan_result
@@ -207,6 +218,13 @@ def main(argv: Sequence[str] | None = None) -> None:
 		default=False,
 		help="Skip report directory function validation",
 	)
+	parser.add_argument(
+		"--no-nullable-filters",
+		action="store_true",
+		default=False,
+		dest="no_nullable_filters",
+		help="Skip nullable Date/Datetime/Data range-filter validation",
+	)
 	parser.add_argument(
 		"--no-orphans",
 		action="store_true",

test_utils/utils/static_analysis/__init__.py

@@ -21,6 +21,7 @@
 from .frontend_validator import FrontendValidationResult, FrontendValidator
 from .hooks_validator import HooksValidationResult, HooksValidator
 from .jinja_validator import JinjaValidationResult, JinjaValidator
+from .nullable_filter_validator import NullableFilterValidationResult, NullableFilterValidator
 from .orphan_detector import OrphanDetector, OrphanResult
 from .patches_validator import PatchesValidationResult, PatchesValidator
 from .path_resolver import PathResolver
@@ -43,6 +44,7 @@ class StaticAnalysisConfig:
 	validate_python_calls: bool = True
 	validate_jinja: bool = True
 	validate_reports: bool = True
+	validate_nullable_filters: bool = True
 	detect_orphans: bool = True
 	min_confidence: int = 80
 	ignore_patterns: list[str] = field(default_factory=list)
@@ -58,6 +60,7 @@ class StaticAnalysisResult:
 	python_call_result: PythonCallValidationResult | None = None
 	jinja_result: JinjaValidationResult | None = None
 	report_result: ReportValidationResult | None = None
+	nullable_filter_result: NullableFilterValidationResult | None = None
 	orphan_result: OrphanResult | None = None
 
 	@property
@@ -104,6 +107,7 @@ def all_warnings(self) -> list[str]:
 			self.python_call_result,
 			self.jinja_result,
 			self.report_result,
+			self.nullable_filter_result,
 		):
 			if r is not None:
 				msgs.extend(r.warnings)
@@ -122,6 +126,7 @@ def r(obj) -> dict | None:
 			"python_calls": r(self.python_call_result),
 			"jinja": r(self.jinja_result),
 			"reports": r(self.report_result),
+			"nullable_filters": r(self.nullable_filter_result),
 			"orphans": r(self.orphan_result),
 		}
 
@@ -301,6 +306,9 @@ def analyze(self) -> StaticAnalysisResult:
 		if self.config.validate_reports:
 			result.report_result = ReportValidator(self.app_path).validate()
 
+		if self.config.validate_nullable_filters:
+			result.nullable_filter_result = NullableFilterValidator().validate(self.app_path)
+
 		if self.config.detect_orphans:
 			from .hooks_validator import PathExtractor
 

test_utils/utils/static_analysis/nullable_filter_validator.py

@@ -0,0 +1,161 @@
+"""
+Flag range filters on nullable Date/Datetime/Time/Data fields with no "is set" guard.
+
+Frappe wraps filters on nullable columns in ``ifnull(col, '')``. That makes
+``''`` a valid operand for comparison operators — ``'' < '2026-01-01'`` is
+string-true on both MariaDB and Postgres — so ``["due_date", "<", nowdate()]``
+also matches rows where ``due_date`` is NULL. Pairing the comparison with
+``["field", "is", "set"]`` (Frappe allows two conditions per fieldname) avoids it.
+"""
+
+import ast
+import json
+from dataclasses import dataclass, field
+from pathlib import Path
+
+RANGE_OPS = frozenset({"<", ">", "<=", ">="})
+NULLABLE_FIELDTYPES = frozenset({"Date", "Datetime", "Data", "Time"})
+
+GET_ALL_FUNCS = frozenset({"get_all", "get_list", "get_count", "exists"})
+
+
+@dataclass
+class NullableFilterValidationResult:
+	errors: list[str] = field(default_factory=list)
+	warnings: list[str] = field(default_factory=list)
+	calls_checked: int = 0
+
+	def to_dict(self) -> dict:
+		return {
+			"errors": self.errors,
+			"warnings": self.warnings,
+			"calls_checked": self.calls_checked,
+		}
+
+
+def _load_nullable_fields(app_path: Path) -> dict[str, set[str]]:
+	"""Map DocType name -> set of non-mandatory Date/Datetime/Data/Time fieldnames."""
+	fields_by_doctype: dict[str, set[str]] = {}
+	for json_file in app_path.rglob("*.json"):
+		if "doctype" not in json_file.parts:
+			continue
+		try:
+			data = json.loads(json_file.read_text(encoding="utf-8"))
+		except (OSError, json.JSONDecodeError):
+			continue
+		if data.get("doctype") != "DocType" or "fields" not in data:
+			continue
+		name = data.get("name") or json_file.stem
+		nullable = {
+			f["fieldname"]
+			for f in data["fields"]
+			if f.get("fieldtype") in NULLABLE_FIELDTYPES and not f.get("reqd") and not f.get("default")
+		}
+		if nullable:
+			fields_by_doctype[name] = nullable
+	return fields_by_doctype
+
+
+def _filter_entries(node: ast.AST) -> list[tuple[ast.AST, int]]:
+	"""Extract literal filter clauses (as AST list/tuple nodes) from a filters argument."""
+	entries: list[tuple[ast.AST, int]] = []
+	if isinstance(node, (ast.List, ast.Tuple)):
+		# Either a single ["field", "op", value] clause or a list of clauses.
+		if node.elts and all(isinstance(e, ast.Constant) for e in node.elts[:2]):
+			entries.append((node, node.lineno))
+		else:
+			for elt in node.elts:
+				entries.extend(_filter_entries(elt))
+	return entries
+
+
+def _clause_field_and_op(clause: ast.AST) -> tuple[str, str] | None:
+	if not isinstance(clause, (ast.List, ast.Tuple)):
+		return None
+	elts = clause.elts
+	if len(elts) == 3 and all(isinstance(e, ast.Constant) for e in (elts[0], elts[1])):
+		return elts[0].value, elts[1].value
+	return None
+
+
+class NullableFilterValidator:
+	def validate(self, app_path: Path) -> NullableFilterValidationResult:
+		result = NullableFilterValidationResult()
+		nullable_fields = _load_nullable_fields(app_path)
+		if not nullable_fields:
+			return result
+
+		for py_file in app_path.rglob("*.py"):
+			if any(p in py_file.parts for p in ("__pycache__", "node_modules", "test", "tests")):
+				continue
+			try:
+				source = py_file.read_text(encoding="utf-8", errors="replace")
+			except OSError:
+				continue
+			try:
+				tree = ast.parse(source, filename=str(py_file))
+			except SyntaxError:
+				continue
+
+			lines = source.splitlines()
+
+			for node in ast.walk(tree):
+				if not isinstance(node, ast.Call):
+					continue
+				func = node.func
+				func_name = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", None)
+				if func_name not in GET_ALL_FUNCS:
+					continue
+
+				# doctype is first positional arg or `doctype=` kwarg.
+				doctype = None
+				if node.args and isinstance(node.args[0], ast.Constant):
+					doctype = node.args[0].value
+				else:
+					for kw in node.keywords:
+						if kw.arg == "doctype" and isinstance(kw.value, ast.Constant):
+							doctype = kw.value.value
+				if doctype not in nullable_fields:
+					continue
+
+				# filters is the 2nd positional arg or `filters=`/`filter=` kwarg.
+				filters_node = None
+				if len(node.args) > 1:
+					filters_node = node.args[1]
+				else:
+					for kw in node.keywords:
+						if kw.arg in ("filters", "filter"):
+							filters_node = kw.value
+				if filters_node is None:
+					continue
+
+				clauses = _filter_entries(filters_node)
+				fields_with_range: dict[str, int] = {}
+				fields_with_guard: set[str] = set()
+				for clause, lineno in clauses:
+					parsed = _clause_field_and_op(clause)
+					if not parsed:
+						continue
+					fname, op = parsed
+					if fname not in nullable_fields[doctype]:
+						continue
+					if op in RANGE_OPS:
+						fields_with_range.setdefault(fname, lineno)
+					elif op == "is":
+						fields_with_guard.add(fname)
+
+				result.calls_checked += 1
+				for fname, lineno in fields_with_range.items():
+					if fname in fields_with_guard:
+						continue
+					line_text = lines[lineno - 1] if 0 < lineno <= len(lines) else ""
+					if "frappe-vulture:ignore" in line_text:
+						continue
+					result.warnings.append(
+						f"{py_file}:{lineno}: {doctype}.{fname} compared with a range operator "
+						f"but not guarded with [\"{fname}\", \"is\", \"set\"] — Frappe wraps "
+						f"nullable columns in ifnull(col, ''), so NULL rows can silently match "
+						f"the comparison"
+					)
+
+		return result