Better type checking; better error messages

Author: dsblank

gramps/plugins/db/dbapi/query_builder.py

@@ -894,6 +894,119 @@ def get_sql_query(self, what, where, order_by, page=None, page_size=None):
         if where:
             where_condition = self.parser.parse_expression(where)
 
+            # Check for == None or != None (should use 'is' or 'is not' instead)
+            from .query_model import CompareExpression, ConstantExpression
+
+            if isinstance(where_condition, CompareExpression):
+                # Check if any comparator is None with == or !=
+                for op, comparator in zip(
+                    where_condition.operators, where_condition.comparators
+                ):
+                    if (
+                        isinstance(comparator, ConstantExpression)
+                        and comparator.value is None
+                    ):
+                        if op == "==":
+                            raise ValueError(
+                                f"Invalid WHERE clause: Cannot use '==' to check for None.\n"
+                                f"  You wrote: {where}\n"
+                                f"  Problem: In SQL, 'value = NULL' is always false (NULL = NULL is false)\n"
+                                f"  Use instead: {where.replace('== None', 'is None')}\n"
+                                f"\n"
+                                f"The 'is None' operator generates 'IS NULL' which correctly checks for NULL values."
+                            )
+                        elif op == "!=":
+                            raise ValueError(
+                                f"Invalid WHERE clause: Cannot use '!=' to check for None.\n"
+                                f"  You wrote: {where}\n"
+                                f"  Problem: In SQL, 'value != NULL' is always false (NULL != NULL is false)\n"
+                                f"  Use instead: {where.replace('!= None', 'is not None')}\n"
+                                f"\n"
+                                f"The 'is not None' operator generates 'IS NOT NULL' which correctly checks for non-NULL values."
+                            )
+                # Also check the left side
+                if (
+                    isinstance(where_condition.left, ConstantExpression)
+                    and where_condition.left.value is None
+                ):
+                    if where_condition.operators[0] == "==":
+                        raise ValueError(
+                            f"Invalid WHERE clause: Cannot use '==' to check for None.\n"
+                            f"  You wrote: {where}\n"
+                            f"  Use instead: {where.replace('None ==', 'None is')}\n"
+                            f"\n"
+                            f"In SQL, 'NULL = value' is always false. Use 'is None' instead."
+                        )
+                    elif where_condition.operators[0] == "!=":
+                        raise ValueError(
+                            f"Invalid WHERE clause: Cannot use '!=' to check for None.\n"
+                            f"  You wrote: {where}\n"
+                            f"  Use instead: {where.replace('None !=', 'None is not')}\n"
+                            f"\n"
+                            f"In SQL, 'NULL != value' is always false. Use 'is not None' instead."
+                        )
+
+            # Validate that WHERE clause is not a bare list comprehension
+            # List comprehensions evaluate to lists, not booleans
+            from .query_model import ListComprehensionExpression
+
+            if isinstance(where_condition, ListComprehensionExpression):
+                raise ValueError(
+                    f"Invalid WHERE clause: List comprehensions are not valid boolean expressions.\n"
+                    f"  You wrote: {where}\n"
+                    f"  Did you mean: any({where})\n"
+                    f"\n"
+                    f"A WHERE clause must evaluate to True/False, but a list comprehension returns a list.\n"
+                    f"Use any([...]) to check if any element matches the condition, or\n"
+                    f"use 'item in array and condition' for array expansion patterns."
+                )
+
+            # Validate that WHERE clause returns a boolean type
+            # Use type inference to check the return type
+            # Note: We only reject clear non-boolean cases (lists, strings at top level)
+            # We allow None (unknown types) and object types (can be NULL checked)
+            if self.enable_type_validation:
+                where_type = self.type_inference.visit(where_condition)
+                # Only validate if we have type information
+                # Reject lists and strings that aren't in a comparison context
+                from .query_model import AttributeExpression, ArrayAccessExpression
+                from typing import get_origin
+
+                is_simple_attribute = isinstance(
+                    where_condition, (AttributeExpression, ArrayAccessExpression)
+                )
+
+                if where_type is not None and is_simple_attribute:
+                    origin = get_origin(where_type)
+
+                    # Reject plain lists - they should use len() or any()
+                    if origin is list or where_type is list:
+                        raise TypeError(
+                            f"Invalid WHERE clause: Expression does not return a boolean.\n"
+                            f"  You wrote: {where}\n"
+                            f"  This returns: list\n"
+                            f"\n"
+                            f"A WHERE clause must evaluate to True/False (boolean), not list.\n"
+                            f"  Use len({where}) > 0 to check if the list is non-empty, or\n"
+                            f"  use any([...]) to check if any element matches a condition."
+                        )
+
+                    # Reject plain strings - they should use comparisons
+                    elif where_type is str:
+                        raise TypeError(
+                            f"Invalid WHERE clause: Expression does not return a boolean.\n"
+                            f"  You wrote: {where}\n"
+                            f"  This returns: string\n"
+                            f"\n"
+                            f"A WHERE clause must evaluate to True/False (boolean), not string.\n"
+                            f"  Use a comparison like {where} == 'value' or 'substring' in {where}."
+                        )
+
+                    # Note: We allow int, float, and object types because:
+                    # - In SQL, numbers can be used as booleans (0=false, non-zero=true)
+                    # - Object types can be NULL checked (NULL=false, non-NULL=true)
+                    # This matches SQL semantics where WHERE column is valid
+
         # Parse order_by clause
         order_by_list = self._parse_order_by(order_by)
 
@@ -1378,6 +1491,18 @@ def _handle_list_comprehensions(self, query: SelectQuery, what) -> SelectQuery:
                         )
                         left_query.union_queries = [right_query]
                         return left_query
+                elif listcomp.array_info.get("type") == "nested_listcomp":
+                    # Check if the inner list comprehension is concatenated
+                    inner_listcomp = listcomp.array_info["inner_listcomp"]
+                    if inner_listcomp.array_info.get("type") == "concatenated":
+                        # Need to generate UNION for nested+concatenated case
+                        left_query, right_query = (
+                            self._build_nested_concatenated_array_queries(
+                                query, listcomp, inner_listcomp
+                            )
+                        )
+                        left_query.union_queries = [right_query]
+                        return left_query
 
         return query
 
@@ -1453,6 +1578,103 @@ def _build_concatenated_array_queries(
 
         return left_query, right_query
 
+    def _build_nested_concatenated_array_queries(
+        self,
+        query: SelectQuery,
+        outer_listcomp: ListComprehensionExpression,
+        inner_listcomp: ListComprehensionExpression,
+    ) -> tuple:
+        """
+        Build left and right queries for nested list comprehensions with concatenated inner arrays.
+
+        Example: [s.surname for s in [name.surname_list for name in [primary_name] + alternate_names]]
+        - Outer: iterates over surname_list results
+        - Inner: iterates over [primary_name] + alternate_names (concatenated)
+        """
+        from .query_model import (
+            AttributeExpression,
+            CallExpression,
+            ConstantExpression,
+        )
+
+        # Create modified inner list comprehensions for left and right sides
+        # Left side: [name.surname_list for name in [primary_name]]
+        # The left side expression from the original concatenated array
+        left_array_attr = inner_listcomp.array_info["left"]
+
+        # For the left side, we need to mark that it should be wrapped in json_array()
+        # We'll use a special marker in array_info
+        left_inner_listcomp = ListComprehensionExpression(
+            expression=inner_listcomp.expression,
+            item_var=inner_listcomp.item_var,
+            array_info={
+                "type": "single",
+                "path": "primary_name",
+                "wrap_in_json_array": True,  # Signal that this needs json_array wrapping
+            },
+            condition=inner_listcomp.condition,
+        )
+
+        # Right side: [name.surname_list for name in alternate_names]
+        right_path = inner_listcomp.array_info["right_path"]
+        right_inner_listcomp = ListComprehensionExpression(
+            expression=inner_listcomp.expression,
+            item_var=inner_listcomp.item_var,
+            array_info={
+                "type": "single",
+                "path": right_path,
+            },
+            condition=inner_listcomp.condition,
+        )
+
+        # Create outer list comprehensions with modified inner ones
+        left_outer_listcomp = ListComprehensionExpression(
+            expression=outer_listcomp.expression,
+            item_var=outer_listcomp.item_var,
+            array_info={
+                "type": "nested_listcomp",
+                "inner_listcomp": left_inner_listcomp,
+            },
+            condition=outer_listcomp.condition,
+        )
+
+        right_outer_listcomp = ListComprehensionExpression(
+            expression=outer_listcomp.expression,
+            item_var=outer_listcomp.item_var,
+            array_info={
+                "type": "nested_listcomp",
+                "inner_listcomp": right_inner_listcomp,
+            },
+            condition=outer_listcomp.condition,
+        )
+
+        # Build left and right queries
+        left_select_expr = SelectExpression(expression=left_outer_listcomp)
+        left_query = SelectQuery(
+            base_table=self.table_name,
+            select_expressions=[left_select_expr],
+            where_condition=query.where_condition,
+            joins=query.joins,
+            order_by=query.order_by,
+            array_expansion=None,
+            limit=query.limit,
+            offset=query.offset,
+        )
+
+        right_select_expr = SelectExpression(expression=right_outer_listcomp)
+        right_query = SelectQuery(
+            base_table=self.table_name,
+            select_expressions=[right_select_expr],
+            where_condition=query.where_condition,
+            joins=query.joins,
+            order_by=query.order_by,
+            array_expansion=None,
+            limit=query.limit,
+            offset=query.offset,
+        )
+
+        return left_query, right_query
+
     def normalize_expression(self, expr: Expression) -> Optional[Expression]:
         """
         Normalize an expression tree by flattening nested AND/OR and optimizing comparisons.

gramps/plugins/db/dbapi/query_model.py

@@ -164,6 +164,9 @@ class ListComprehensionExpression(Expression):
     item_var: str  # Iteration variable name
     array_info: dict  # {'type': 'single'/'concatenated', 'path': str, etc.}
     condition: Optional[Expression] = None  # Optional filter condition
+    item_type: Optional[type] = (
+        None  # Type of items being iterated (e.g., Name, Surname)
+    )
 
     def __repr__(self):
         return f"ListComp({self.item_var} in {self.array_info})"

gramps/plugins/db/dbapi/query_parser.py

@@ -522,6 +522,9 @@ def _parse_listcomp(self, node: ast.ListComp) -> ListComprehensionExpression:
         # Extract array info
         array_info = self._extract_array_info(iter_node)
 
+        # Infer the item type from the array being iterated
+        item_type = self._infer_list_item_type(array_info)
+
         # Extract expression with item_var context
         # Temporarily set item_var so that the expression can reference it
         old_item_var = self.item_var
@@ -544,6 +547,7 @@ def _parse_listcomp(self, node: ast.ListComp) -> ListComprehensionExpression:
             item_var=item_var,
             array_info=array_info,
             condition=condition,
+            item_type=item_type,
         )
 
     def _extract_array_info(self, iter_node: ast.AST) -> dict:
@@ -603,8 +607,83 @@ def _extract_array_info(self, iter_node: ast.AST) -> dict:
                         "outer_item_var": self.item_var,
                     }
 
+        # Check for nested list comprehension
+        if isinstance(iter_node, ast.ListComp):
+            # Parse the inner list comprehension recursively
+            inner_listcomp = self._parse_listcomp(iter_node)
+            return {
+                "type": "nested_listcomp",
+                "inner_listcomp": inner_listcomp,
+            }
+
         raise ValueError(f"Could not extract array info from: {iter_node}")
 
+    def _infer_list_item_type(self, array_info: dict) -> Optional[type]:
+        """
+        Infer the type of items in a list comprehension from the array info.
+
+        Args:
+            array_info: Dictionary containing array information
+
+        Returns:
+            Type of items, or None if cannot be determined
+        """
+        array_type = array_info.get("type")
+
+        if array_type == "single":
+            # Simple array like person.alternate_names
+            array_path = array_info.get("path", "")
+            return self.type_inference.infer_array_item_type(
+                self.table_name, array_path
+            )
+
+        elif array_type == "concatenated":
+            # Concatenated array like [person.primary_name] + person.alternate_names
+            # Both sides should have the same item type
+            right_path = array_info.get("right_path", "")
+            return self.type_inference.infer_array_item_type(
+                self.table_name, right_path
+            )
+
+        elif array_type == "nested":
+            # Nested iteration like "for surname in name.surname_list"
+            # We need to look up the type from the outer item variable's type
+            # For now, we don't have the outer item's type stored, so return None
+            # This could be enhanced later
+            return None
+
+        elif array_type == "nested_listcomp":
+            # Nested list comprehension - the item type is the type of the inner comprehension's expression
+            # For example: [name.surname_list for name in ...] returns List[Surname]
+            # So the outer iteration "for s in [name.surname_list ...]" iterates over Surname items
+            inner_listcomp = array_info.get("inner_listcomp")
+            if (
+                inner_listcomp
+                and inner_listcomp.expression
+                and inner_listcomp.item_type
+            ):
+                # Push the inner item type onto the stack so we can infer the expression type
+                self.type_inference._listcomp_item_type_stack.append(
+                    inner_listcomp.item_type
+                )
+                try:
+                    # Infer the type of the inner expression (e.g., name.surname_list)
+                    expr_type = self.type_inference.visit(inner_listcomp.expression)
+                    if expr_type:
+                        # If the expression is a list, extract the item type
+                        item_type = self.type_inference._extract_list_item_type(
+                            expr_type
+                        )
+                        if item_type:
+                            return item_type
+                        # If it's not a list type, return the expression type itself
+                        return expr_type
+                finally:
+                    # Pop the item type from the stack
+                    self.type_inference._listcomp_item_type_stack.pop()
+
+        return None
+
     def detect_array_expansion(self, expr_str: str) -> Optional[ArrayExpansion]:
         """Detect array expansion pattern in expression."""
         try: