fix(codex): fix strings combiner

closes #733

Signed-off-by: azjezz <[email protected]>

Author: azjezz

crates/analyzer/tests/cases/issue_733.php

@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+final class Package
+{
+    private const string EMPTY_VALUE = 'k. A.';
+
+    /** @var numeric-string|self::EMPTY_VALUE */
+    private string $quantity1;
+
+    /** @var numeric-string|'Empty' */
+    private string $quantity2;
+
+    public function __construct(int|null $quantity1, int|null $quantity2)
+    {
+        $this->quantity1 = $quantity1 !== null ? (string) $quantity1 : self::EMPTY_VALUE;
+        $this->quantity2 = $quantity2 !== null ? (string) $quantity2 : 'Empty';
+    }
+
+    public function quantity1(): int|null
+    {
+        return $this->quantity1 !== self::EMPTY_VALUE ? (int) $this->quantity1 : null;
+    }
+
+    public function quantity2(): int|null
+    {
+        return $this->quantity2 !== 'Empty' ? (int) $this->quantity2 : null;
+    }
+}

crates/analyzer/tests/mod.rs

@@ -441,6 +441,7 @@ test_case!(issue_722);
 test_case!(issue_725);
 test_case!(issue_728_symfony_reference);
 test_case!(issue_731);
+test_case!(issue_733);
 test_case!(issue_736);
 test_case!(issue_737);
 test_case!(issue_739);

crates/codex/src/ttype/atomic/scalar/mod.rs

@@ -108,7 +108,7 @@ impl TScalar {
     /// Creates the `numeric-string` type.
     #[inline]
     pub const fn numeric_string() -> Self {
-        TScalar::String(TString::general_with_props(true, false, false, true))
+        TScalar::String(TString::general_with_props(true, false, false, false))
     }
 
     /// Creates the general `string` type.

crates/codex/src/ttype/atomic/scalar/string.rs

@@ -87,10 +87,9 @@ impl TString {
         is_numeric: bool,
         is_truthy: bool,
         mut is_non_empty: bool,
-        mut is_lowercase: bool,
+        is_lowercase: bool,
     ) -> Self {
         is_non_empty |= is_numeric || is_truthy;
-        is_lowercase |= is_numeric;
 
         Self { literal, is_numeric, is_truthy, is_non_empty, is_lowercase }
     }
@@ -278,8 +277,8 @@ impl TString {
             literal: if retain_literal { self.literal.clone() } else { None },
             is_numeric: true,
             is_truthy: self.is_truthy,
-            is_non_empty: true,
-            is_lowercase: true, // Numeric strings are considered lowercase
+            is_non_empty: true, // Numeric strings are always non-empty
+            is_lowercase: self.is_lowercase,
         }
     }
 }

crates/codex/src/ttype/combiner.rs

@@ -926,12 +926,35 @@ fn scrape_type_properties(
     if let TAtomic::Scalar(TScalar::String(mut string_scalar)) = atomic {
         if let Some(existing_string_type) = combination.value_types.get_mut(&*ATOM_STRING) {
             if let TAtomic::Scalar(TScalar::String(existing_string_type)) = existing_string_type {
-                *existing_string_type = combine_string_scalars(existing_string_type, string_scalar);
+                if let Some(lit_atom) = string_scalar.get_known_literal_atom() {
+                    let lit_value = lit_atom.as_str();
+                    let is_incompatible = (existing_string_type.is_numeric && !str_is_numeric(lit_value))
+                        || (existing_string_type.is_truthy && (lit_value.is_empty() || lit_value == "0"))
+                        || (existing_string_type.is_non_empty && lit_value.is_empty())
+                        || (existing_string_type.is_lowercase && lit_value.chars().any(|c| c.is_uppercase()));
+
+                    if is_incompatible {
+                        combination.literal_strings.insert(lit_atom);
+                    } else {
+                        *existing_string_type = combine_string_scalars(existing_string_type, string_scalar);
+                    }
+                } else {
+                    *existing_string_type = combine_string_scalars(existing_string_type, string_scalar);
+                }
             };
         } else if let Some(atom) = string_scalar.get_known_literal_atom() {
             combination.literal_strings.insert(atom);
         } else {
-            if string_scalar.is_truthy || string_scalar.is_non_empty || string_scalar.is_numeric {
+            // When we have a constrained string type (like numeric-string) and literals,
+            // we need to decide whether to merge them or keep them separate.
+            // If the non-literal is numeric-string, keep non-numeric literals separate.
+            let mut literals_to_keep = Vec::new();
+
+            if string_scalar.is_truthy
+                || string_scalar.is_non_empty
+                || string_scalar.is_numeric
+                || string_scalar.is_lowercase
+            {
                 for value in &combination.literal_strings {
                     if value.is_empty() {
                         string_scalar.is_non_empty = false;
@@ -942,12 +965,23 @@ fn scrape_type_properties(
                         string_scalar.is_truthy = false;
                     }
 
-                    string_scalar.is_numeric = string_scalar.is_numeric && str_is_numeric(value);
+                    // If the string is numeric but the literal is not, keep the literal separate
+                    if string_scalar.is_numeric && !str_is_numeric(value) {
+                        literals_to_keep.push(*value);
+                    } else {
+                        string_scalar.is_numeric = string_scalar.is_numeric && str_is_numeric(value);
+                    }
+
+                    string_scalar.is_lowercase = string_scalar.is_lowercase && value.chars().all(|c| !c.is_uppercase());
                 }
             }
 
             combination.value_types.insert(*ATOM_STRING, TAtomic::Scalar(TScalar::String(string_scalar)));
+
             combination.literal_strings.clear();
+            for lit in literals_to_keep {
+                combination.literal_strings.insert(lit);
+            }
         }
 
         return;