JDT Clean-up Use String.replace() instead of String.replaceAll() when

[vogella]

possible

In https://github.com/eclipse-platform/eclipse.platform.ui/issues/3517 we discuss which automatic clean-ups we should add. This is the result of a manual run in eclipse-platform.ui to see if this clean-up has issues. Also by running it manually before enabling the clean-ups we avoid creating PR per bundle.

[laeubi]

So do you see any issues with that or not?

[vogella]

@jjohnstn I think the change in AbstractTemplatePage are incorrect:

---

// Before (CORRECT): pattern.replaceAll("\$", "\$\$")

// After (INCORRECT): pattern.replace("$", "$$")

Problem: The original code was escaping $ for regex replacement strings, not for regex patterns. In replaceAll(), the replacement string has special meaning for $ (backreferences like $1, $2). To insert a literal $, you need \$\$ (which becomes $$ after Java string escaping).

However, String.replace() doesn't interpret $ specially in the replacement string. So while $$ will insert two literal dollar signs, the original intent was to escape one dollar sign for template syntax.

Impact: This changes behavior - templates with $ will now have $$ instead of being properly escaped for the template engine.

---

If you agree, I can open an issue in JDT UI

[vogella]

Converting to draft so that this is not merged.

[merks]

I'm a big confused....

This method

	public static void main(String[] args) {
		for (var pattern : List.of("$", "$$", "${}")) {
			System.err.println(pattern.replaceAll("\\$", "\\$\\$"));
			System.err.println(pattern.replace("$", "$$"));
			System.err.println();
		}
	}

prints this result

$$
$$

$$$$
$$$$

$${}
$${}

So I'm tying to imagine a string for which they would print out two different results...

[github-actions[bot]]

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 19m 48s ⏱️ + 2m 29s  8 234 tests ±0   7 985 ✅ ±0  249 💤 ±0  0 ❌ ±0  23 622 runs  ±0  22 828 ✅ ±0  794 💤 ±0  0 ❌ ±0 

Results for commit c9bf7e62. ± Comparison against base commit a3fa0658.

:recycle: This comment has been updated with latest results.

[vogella]

I'm a big confused....

This method

	public static void main(String[] args) {
		for (var pattern : List.of("$", "$$", "${}")) {
			System.err.println(pattern.replaceAll("\\$", "\\$\\$"));
			System.err.println(pattern.replace("$", "$$"));
			System.err.println();
		}
	}

prints this result

$$
$$

$$$$
$$$$

$${}
$${}

So I'm tying to imagine a string for which they would print out two different results...

You're absolutely right to be confused - I made an error! Let me verify this myself:

import java.util.List;

    public class TestStringReplace {
        public static void main(String[] args) {
            System.out.println("Testing different patterns:");
            for (var pattern : List.of("$", "$$", "${}", "$1", "test$middle", "$start", "end$")) {
                String replaceAllResult = pattern.replaceAll("\\$", "\\$\\$");
                String replaceResult = pattern.replace("$", "$$");
                System.out.println("Input: '" + pattern + "'");
                System.out.println("  replaceAll: '" + replaceAllResult + "'");
                System.out.println("  replace:    '" + replaceResult + "'");
                System.out.println("  Same? " + replaceAllResult.equals(replaceResult));
                System.out.println();
            }
        }
    }

I was wrong. @jjohnstn please ignore my request.

I would still wait this converting this to a real PR again, as IMHO we should not apply such clean-ups shortly before a release.