Skip to content

[Repo Assist] [JS/TS + Python] Fix String.Contains ignoring StringComparison argument#4485

Draft
github-actions[bot] wants to merge 1 commit intomainfrom
repo-assist/fix-contains-stringcomparison-5cf9b6dcbc09f14a
Draft

[Repo Assist] [JS/TS + Python] Fix String.Contains ignoring StringComparison argument#4485
github-actions[bot] wants to merge 1 commit intomainfrom
repo-assist/fix-contains-stringcomparison-5cf9b6dcbc09f14a

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Apr 4, 2026

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

String.Contains(value, StringComparison) was silently discarding the StringComparison argument and always performing a case-sensitive search. This produced incorrect results, e.g.:

"ABC".Contains("b", StringComparison.OrdinalIgnoreCase) // returns false (wrong — should be true)

Root Cause

In both Replacements.fs (JS/TS) and Python/Replacements.fs, the Contains pattern matched arg :: _ and emitted a compile-time warning plus a case-sensitive indexOf/find call regardless of the second argument.

Fix

JS/TS:

  • Added contains(str, pattern, ic) to src/fable-library-ts/String.ts, using the existing cmp() helper (same pattern as startsWith/endsWith)
  • Updated Replacements.fs to route Contains(value, comparison) to String.contains from the library

Python:

  • Added contains(string, value, comparison) to src/fable-library-py/fable_library/string_.py — pure Python implementation that handles all ignore-case StringComparison variants
  • Updated Python/Replacements.fs similarly

Tests:

  • Added String.Contains with StringComparison works to tests/Js/Main/StringTests.fs
  • Added test String.Contains with StringComparison works to tests/Python/TestString.fs

Trade-offs

The JS contains function uses a character-by-character scan for culture-sensitive comparisons (same strategy as startsWith/endsWith). For the common OrdinalIgnoreCase case the loop's cmp() call uses simple toLowerCase(), which is fast in practice.

Note

🔒 Integrity filter blocked 84 items

The following items were blocked because they don't meet the GitHub integrity level.

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Repo Assist · ● 7.6M ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f

…ython targets

When calling String.Contains with a StringComparison argument (e.g.
StringComparison.OrdinalIgnoreCase), Fable was silently discarding the
second argument and always doing a case-sensitive search. This caused
incorrect results: e.g. "ABC".Contains("b", StringComparison.OrdinalIgnoreCase)
returned false instead of true.

Fix:
- Add contains(str, pattern, ic) to fable-library-ts/String.ts using
  the existing cmp() helper (same pattern as startsWith/endsWith)
- Add contains(string, value, comparison) to fable_library/string_.py
  (pure Python, handles ignore-case variants)
- Update Replacements.fs (JS/TS) and Python/Replacements.fs to route
  Contains(value, comparison) to the new library function instead of
  warning and ignoring the comparison argument
- Add regression tests to tests/Js/Main/StringTests.fs and
  tests/Python/TestString.fs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions bot added automation Automated changes repo-assist Created by Repo Assist labels Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation Automated changes repo-assist Created by Repo Assist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants