mirror of
https://github.com/dart-lang/sdk
synced 2024-10-14 11:03:19 +00:00
[dart2wasm] Fix JSStringImpl trim methods, add tests
Fix trim methods returning a new string (instead of the argument) when there's nothing to trim. A second bug fixed in NEL handling in `trim`: the last line should be `result.substring(...)` instead of `this.substring(...)`. The bug wasn't caught by any of the tests: NEL needs to be handled specially on the browser, but mixing NEL with other whitespace wasn't tested in the existing tests `string_trim_test` and `string_trimlr_test`, so a new test file added with this case. Change-Id: Ibf84e185de1b26c9811e3ea58c2ad3f223da8515 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363081 Reviewed-by: Martin Kustermann <kustermann@google.com> Commit-Queue: Ömer Ağacan <omersa@google.com> Reviewed-by: Lasse Nielsen <lrn@google.com>
This commit is contained in:
parent
166e46575f
commit
55f2754d6b
|
@ -328,6 +328,7 @@ final class JSStringImpl implements String {
|
|||
String substring(int start, [int? end]) {
|
||||
end ??= length;
|
||||
RangeErrorUtils.checkValidRangePositiveLength(start, end, length);
|
||||
if (start == end) return "";
|
||||
return JSStringImpl(_jsSubstring(toExternRef, start, end));
|
||||
}
|
||||
|
||||
|
@ -441,73 +442,96 @@ final class JSStringImpl implements String {
|
|||
return index;
|
||||
}
|
||||
|
||||
// dart2wasm can't use JavaScript trim directly,
|
||||
// because JavaScript does not trim
|
||||
// the NEXT LINE (NEL) character (0x85).
|
||||
// dart2wasm can't use JavaScript trim directly, because JavaScript does not
|
||||
// trim the NEXT LINE (NEL) character (0x85).
|
||||
@override
|
||||
String trim() {
|
||||
// Start by doing JS trim. Then check if it leaves a NEL at
|
||||
// either end of the string.
|
||||
final length = this.length;
|
||||
if (length == 0) return this;
|
||||
|
||||
// Start by doing JS trim. Then check if it leaves a NEL at either end of
|
||||
// the string.
|
||||
final result =
|
||||
JSStringImpl(js.JS<WasmExternRef?>('s => s.trim()', toExternRef));
|
||||
final resultLength = result.length;
|
||||
if (resultLength == 0) return result;
|
||||
int firstCode = result._codeUnitAtUnchecked(0);
|
||||
|
||||
// Check NEL on the left.
|
||||
final int firstCode = result._codeUnitAtUnchecked(0);
|
||||
int startIndex = 0;
|
||||
if (firstCode == nelCodeUnit) {
|
||||
startIndex = _skipLeadingWhitespace(result, 1);
|
||||
if (startIndex == resultLength) return "";
|
||||
}
|
||||
|
||||
// Check NEL on the right.
|
||||
int endIndex = resultLength;
|
||||
// We know that there is at least one character that is non-whitespace.
|
||||
// Therefore we don't need to verify that endIndex > startIndex.
|
||||
int lastCode = result.codeUnitAt(endIndex - 1);
|
||||
final int lastCode = result.codeUnitAt(endIndex - 1);
|
||||
if (lastCode == nelCodeUnit) {
|
||||
endIndex = _skipTrailingWhitespace(result, endIndex - 1);
|
||||
}
|
||||
if (startIndex == 0 && endIndex == resultLength) return result;
|
||||
return substring(startIndex, endIndex);
|
||||
|
||||
if (startIndex == 0 && endIndex == resultLength) {
|
||||
return length == resultLength ? this : result;
|
||||
}
|
||||
|
||||
return result.substring(startIndex, endIndex);
|
||||
}
|
||||
|
||||
// dart2wasm can't use JavaScript trimLeft directly because it does not trim
|
||||
// the NEXT LINE character (0x85).
|
||||
// the NEXT LINE (NEL) character (0x85).
|
||||
@override
|
||||
String trimLeft() {
|
||||
// Start by doing JS trim. Then check if it leaves a NEL at
|
||||
// the beginning of the string.
|
||||
final length = this.length;
|
||||
if (length == 0) return this;
|
||||
|
||||
// Start by doing JS trim. Then check if it leaves a NEL at the beginning
|
||||
// of the string.
|
||||
int startIndex = 0;
|
||||
final result =
|
||||
JSStringImpl(js.JS<WasmExternRef?>('s => s.trimLeft()', toExternRef));
|
||||
final resultLength = result.length;
|
||||
if (resultLength == 0) return result;
|
||||
|
||||
// Check NEL.
|
||||
int firstCode = result._codeUnitAtUnchecked(0);
|
||||
if (firstCode == nelCodeUnit) {
|
||||
startIndex = _skipLeadingWhitespace(result, 1);
|
||||
}
|
||||
if (startIndex == 0) return result;
|
||||
if (startIndex == resultLength) return "";
|
||||
|
||||
if (startIndex == 0) {
|
||||
return resultLength == length ? this : result;
|
||||
}
|
||||
|
||||
return result.substring(startIndex);
|
||||
}
|
||||
|
||||
// dart2wasm can't use JavaScript trimRight directly because it does not trim
|
||||
// the NEXT LINE character (0x85).
|
||||
// the NEXT LINE (NEL) character (0x85).
|
||||
@override
|
||||
String trimRight() {
|
||||
final length = this.length;
|
||||
if (length == 0) return this;
|
||||
|
||||
// Start by doing JS trim. Then check if it leaves a NEL at the end of the
|
||||
// string.
|
||||
final result =
|
||||
JSStringImpl(js.JS<WasmExternRef?>('s => s.trimRight()', toExternRef));
|
||||
final resultLength = result.length;
|
||||
if (resultLength == 0) return result;
|
||||
|
||||
int endIndex = resultLength;
|
||||
if (endIndex == 0) return result;
|
||||
int lastCode = result.codeUnitAt(endIndex - 1);
|
||||
if (lastCode == nelCodeUnit) {
|
||||
endIndex = _skipTrailingWhitespace(result, endIndex - 1);
|
||||
}
|
||||
|
||||
if (endIndex == resultLength) return result;
|
||||
if (endIndex == 0) return "";
|
||||
if (endIndex == resultLength) {
|
||||
return resultLength == length ? this : result;
|
||||
}
|
||||
|
||||
return result.substring(0, endIndex);
|
||||
}
|
||||
|
||||
|
|
34
tests/corelib/string_trim_nel_test.dart
Normal file
34
tests/corelib/string_trim_nel_test.dart
Normal file
|
@ -0,0 +1,34 @@
|
|||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
// NEXT LINE (NEL, 0x85) character is special in that some Unicode versions do
|
||||
// not include it as a whitespace character.
|
||||
//
|
||||
// However all Dart backends handle it as a whitespace character. dart2js and
|
||||
// dart2wasm do this by a calling JS (`String.prototype.trim` and its
|
||||
// left/right variants) and doing post-processing. To check these
|
||||
// implementations tests in this file mix the SPACE character with NEL in
|
||||
// various positions.
|
||||
//
|
||||
// string_trim_lr_test.dart tests various NEL characters on the left and right,
|
||||
// but they do not mix NEL with other whitespace characters.
|
||||
// string_trim_test.dart tests do not test NEL.
|
||||
|
||||
import "package:expect/expect.dart";
|
||||
|
||||
const int space = 0x20;
|
||||
const int nel = 0x85;
|
||||
const int h = 0x68;
|
||||
const int i = 0x69;
|
||||
|
||||
void main() {
|
||||
Expect.equals(
|
||||
"hi", String.fromCharCodes([space, nel, space, h, i]).trimLeft());
|
||||
Expect.equals(
|
||||
"hi", String.fromCharCodes([h, i, space, nel, space]).trimRight());
|
||||
Expect.equals(
|
||||
"hi",
|
||||
String.fromCharCodes([space, nel, space, h, i, space, nel, space])
|
||||
.trim());
|
||||
}
|
Loading…
Reference in a new issue