Normalization of Line Terminators in Xbase strings

[LorenzoBettini]

Closes #2293

[LorenzoBettini]

Looks like I haven't gotten the replacement correctly: it also leads to this unwanted difference:

I thought this test would check that efa5aa1#diff-7217ccbb0669983e16ca897570a543e7ee1bfb23a156c11fe9425cc5db1536baR2210
but maybe this only checks the input string "\r", while we need to check "\r", that is when the user enters the 2 characters "\r", while I'd like to remove the single character '\r'.

[LorenzoBettini]

OK, this fadf9c7 reproduces the problem and fixes it

[LorenzoBettini]

@szarnekow the build fails. I guess I haven't override all the needed methods in the custom value converter?

[szarnekow]

It's an interesting case. I guess we want to treat an explicit \r in the string literal different from a regular windows newline. I've to think about it.

[szarnekow]

cf

	public static void main(String[] args) {
		String s = """
				some\r\nstring""";
		System.out.println(s);
		System.out.println(s.length());
		System.out.println('\r' == s.charAt(4));
	}

[LorenzoBettini]

@szarnekow a quick thought: I guess the original string should be normalized before passing it to super, not afterward as I'm doing now

[LorenzoBettini]

And the current implementation is still not correct, since we have this result:

	@Test
	public void testStringLiteralWithCarriageReturn3_Issue2293() throws Exception {
		compilesTo(
				"{\"astring\".toString.replaceAll(\"\\r\\n\", \"\\n\");}",
				"return \"astring\".toString().replaceAll(\"\\n\", \"\\n\");");
	}

The "\r" from the original source code is removed, which is wrong!

[LorenzoBettini]

Passing the already normalized string to super:

	public static class WindowsEOLsAwareSTRINGValueConverter extends STRINGValueConverter {
		@Override
		public String toValue(String string, INode node) {
			return super.toValue(Strings.toUnixLineSeparator(string), node);
		}
	}

leads to the expected test result:

	@Test
	public void testStringLiteralWithCarriageReturn3_Issue2293() throws Exception {
		compilesTo(
				"{\"astring\".toString.replaceAll(\"\\r\\n\", \"\\n\");}",
				"return \"astring\".toString().replaceAll(\"\\r\\n\", \"\\n\");");
	}

[LorenzoBettini]

@szarnekow concerning the failing tests in Xtend, I think I found the culprit: the way I customized the Xbase string converter did not allow for customizations, and the Xtend customizations were not picked anymore. This should do: 9b8cc60

P.S., do you think it's worthwhile to rename the Xtend StringValueConverter to XtendStringValueConverter? That would allow for easier discovery.

[LorenzoBettini]

@szarnekow I rebased the PR

This is still to be done if you think it's feasible:

P.S., do you think it's worthwhile to rename the Xtend StringValueConverter to XtendStringValueConverter? That would allow for easier discovery.

[szarnekow]

feasible

Yes

XtendStringValueConverter

Yes

[LorenzoBettini]

@szarnekow rebased and renamed the Xtend string value converter.
As for me, this PR would be ready.

[szarnekow]

Please add tests here that use the Java string literal \\r\\n and \\n respectively. These should remain untouched, since they don't rely on the line endings of the compilation unit but use backlash-r and backslash-n respectively.

[LorenzoBettini]

@szarnekow this one d7c67a3 ?

Yes, we have a duplicated AbstractXbaseEvaluationTest ;)

[szarnekow]

I knew that I saw it somewhere ... damn it :)

Thanks!

[szarnekow]

Please add a reminder for the N&N