Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CoreLib] String.splitMap #59686

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/dev_compiler/test/nullable_inference_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ void main() {
s.replaceRange(1, 2, s);
s.split(s);
s.splitMapJoin(s, onMatch: (_) => s, onNonMatch: (_) => s);
s.splitMap(s, onMatch: (_) => s, onNonMatch: (_) => s);
s.startsWith(s);
s.substring(1);
s.toLowerCase();
Expand Down
4 changes: 4 additions & 0 deletions pkg/linter/lib/src/rules/null_closures.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ final Map<String, Set<NonNullableFunction>>
NonNullableFunction('dart.core', 'String', 'splitMapJoin',
named: ['onMatch', 'onNonMatch']),
},
'splitMap': {
NonNullableFunction('dart.core', 'Iterable', 'splitMap',
named: ['onMatch', 'onNonMatch']),
},
'takeWhile': {
NonNullableFunction('dart.core', 'Iterable', 'takeWhile', positional: [0]),
},
Expand Down
1 change: 1 addition & 0 deletions pkg/linter/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7037,6 +7037,7 @@ LintCode:
* `String.replaceAllMapped` at the 1st positional parameter
* `String.replaceFirstMapped` at the 1st positional parameter
* `String.splitMapJoin` at the named parameters `onMatch` and `onNonMatch`
* `String.splitMap` at the named parameter `onMatch` and `onNonMatch`

**BAD:**
```dart
Expand Down
2 changes: 1 addition & 1 deletion pkg/linter/tool/machine/rules.json
Original file line number Diff line number Diff line change
Expand Up @@ -1532,7 +1532,7 @@
"incompatible": [],
"sets": [],
"fixStatus": "hasFix",
"details": "**DON'T** pass `null` as an argument where a closure is expected.\n\nOften a closure that is passed to a method will only be called conditionally,\nso that tests and \"happy path\" production calls do not reveal that `null` will\nresult in an exception being thrown.\n\nThis rule only catches null literals being passed where closures are expected\nin the following locations:\n\n#### Constructors\n\n* From `dart:async`\n * `Future` at the 0th positional parameter\n * `Future.microtask` at the 0th positional parameter\n * `Future.sync` at the 0th positional parameter\n * `Timer` at the 0th positional parameter\n * `Timer.periodic` at the 1st positional parameter\n* From `dart:core`\n * `List.generate` at the 1st positional parameter\n\n#### Static functions\n\n* From `dart:async`\n * `scheduleMicrotask` at the 0th positional parameter\n * `Future.doWhile` at the 0th positional parameter\n * `Future.forEach` at the 0th positional parameter\n * `Future.wait` at the named parameter `cleanup`\n * `Timer.run` at the 0th positional parameter\n\n#### Instance methods\n\n* From `dart:async`\n * `Future.then` at the 0th positional parameter\n * `Future.complete` at the 0th positional parameter\n* From `dart:collection`\n * `Queue.removeWhere` at the 0th positional parameter\n * `Queue.retain\n * `Iterable.firstWhere` at the 0th positional parameter, and the named\n parameter `orElse`\n * `Iterable.forEach` at the 0th positional parameter\n * `Iterable.fold` at the 1st positional parameter\n * `Iterable.lastWhere` at the 0th positional parameter, and the named\n parameter `orElse`\n * `Iterable.map` at the 0th positional parameter\n * `Iterable.reduce` at the 0th positional parameter\n * `Iterable.singleWhere` at the 0th positional parameter, and the named\n parameter `orElse`\n * `Iterable.skipWhile` at the 0th positional parameter\n * `Iterable.takeWhile` at the 0th positional parameter\n * `Iterable.where` at the 0th positional parameter\n * `List.removeWhere` at the 0th positional parameter\n * `List.retainWhere` at the 0th positional parameter\n * `String.replaceAllMapped` at the 1st positional parameter\n * `String.replaceFirstMapped` at the 1st positional parameter\n * `String.splitMapJoin` at the named parameters `onMatch` and `onNonMatch`\n\n**BAD:**\n```dart\n[1, 3, 5].firstWhere((e) => e.isOdd, orElse: null);\n```\n\n**GOOD:**\n```dart\n[1, 3, 5].firstWhere((e) => e.isOdd, orElse: () => null);\n```",
"details": "**DON'T** pass `null` as an argument where a closure is expected.\n\nOften a closure that is passed to a method will only be called conditionally,\nso that tests and \"happy path\" production calls do not reveal that `null` will\nresult in an exception being thrown.\n\nThis rule only catches null literals being passed where closures are expected\nin the following locations:\n\n#### Constructors\n\n* From `dart:async`\n * `Future` at the 0th positional parameter\n * `Future.microtask` at the 0th positional parameter\n * `Future.sync` at the 0th positional parameter\n * `Timer` at the 0th positional parameter\n * `Timer.periodic` at the 1st positional parameter\n* From `dart:core`\n * `List.generate` at the 1st positional parameter\n\n#### Static functions\n\n* From `dart:async`\n * `scheduleMicrotask` at the 0th positional parameter\n * `Future.doWhile` at the 0th positional parameter\n * `Future.forEach` at the 0th positional parameter\n * `Future.wait` at the named parameter `cleanup`\n * `Timer.run` at the 0th positional parameter\n\n#### Instance methods\n\n* From `dart:async`\n * `Future.then` at the 0th positional parameter\n * `Future.complete` at the 0th positional parameter\n* From `dart:collection`\n * `Queue.removeWhere` at the 0th positional parameter\n * `Queue.retain\n * `Iterable.firstWhere` at the 0th positional parameter, and the named\n parameter `orElse`\n * `Iterable.forEach` at the 0th positional parameter\n * `Iterable.fold` at the 1st positional parameter\n * `Iterable.lastWhere` at the 0th positional parameter, and the named\n parameter `orElse`\n * `Iterable.map` at the 0th positional parameter\n * `Iterable.reduce` at the 0th positional parameter\n * `Iterable.singleWhere` at the 0th positional parameter, and the named\n parameter `orElse`\n * `Iterable.skipWhile` at the 0th positional parameter\n * `Iterable.takeWhile` at the 0th positional parameter\n * `Iterable.where` at the 0th positional parameter\n * `List.removeWhere` at the 0th positional parameter\n * `List.retainWhere` at the 0th positional parameter\n * `String.replaceAllMapped` at the 1st positional parameter\n * `String.replaceFirstMapped` at the 1st positional parameter\n * `String.splitMapJoin` at the named parameters `onMatch` and `onNonMatch` * `String.splitMap` at the named parameters `onMatch` and `onNonMatch`\n\n**BAD:**\n```dart\n[1, 3, 5].firstWhere((e) => e.isOdd, orElse: null);\n```\n\n**GOOD:**\n```dart\n[1, 3, 5].firstWhere((e) => e.isOdd, orElse: () => null);\n```",
"sinceDartSdk": "2.0"
},
{
Expand Down
98 changes: 98 additions & 0 deletions sdk/lib/_internal/js_dev_runtime/private/js_string.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,104 @@ final class JSString extends Interceptor
return stringReplaceAllFuncUnchecked(this, from, onMatch, onNonMatch);
}

@notNull
Iterable<T> splitMap<T>(
Pattern pattern, {
T Function(Match match)? onMatch,
T Function(String nonMatch)? onNonMatch,
}) {
return stringSplitMapUnchecked(this, pattern, onMatch, onNonMatch);
}

Iterable<T> stringSplitMapUnchecked<T>(String receiver, Pattern pattern,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just style comments, not sure this is the design we'll do.)
Should be private. All helper functions should.

T Function(Match)? onMatch, T Function(String)? onNonMatch) {
onMatch ??= (match) => match[0]! as T;
onNonMatch ??= (string) => string as T;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely not sound. If the parameters are optional, I'd emit nothing for a match/non-match if onMatch/onNonMatch is null.
(And I'd probably make them named parameters today.)

if (pattern is String) {
return stringSplitStringMapUnchecked(
receiver, pattern, onMatch, onNonMatch);
}
if (pattern is JSSyntaxRegExp) {
return stringSplitJSRegExpMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}
return stringSplitGeneralMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}

Iterable<T> stringSplitStringMapUnchecked<T>(String receiver, String pattern,
T Function(Match) onMatch, T Function(String) onNonMatch) {
if (pattern.isEmpty) {
return stringSplitEmptyMapUnchecked(receiver, onMatch, onNonMatch);
}
List<T> result = <T>[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be lazy, it shouldn't build a List eagerly and return it as an Iterable.

int startIndex = 0;
int patternLength = pattern.length;
while (true) {
int position = stringIndexOfStringUnchecked(receiver, pattern, startIndex);
if (position == -1) {
break;
}
result.add(onNonMatch(receiver.substring(startIndex, position)));
result.add(onMatch(StringMatch(position, receiver, pattern)));
startIndex = position + patternLength;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

Iterable<T> stringSplitEmptyMapUnchecked<T>(
String receiver, T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int length = receiver.length;
int i = 0;
result.add(onNonMatch(""));
while (i < length) {
result.add(onMatch(StringMatch(i, receiver, "")));
// Special case to avoid splitting a surrogate pair.
Copy link
Member

@lrhn lrhn Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A string.split("") would split surrogate pairs. So would string.split(RegExp(r'(?:)')) without the unicode:true flag.

(It'd be nice if it didn't, but then it'd be even niceer if it didn't break grapheme clusters either.)

(I can see that splitMapJoin does the same thing. Probably my bad, it shouldn't have.)

int code = receiver.codeUnitAt(i);
if ((code & ~0x3FF) == 0xD800 && length > i + 1) {
// Leading surrogate;
code = receiver.codeUnitAt(i + 1);
if ((code & ~0x3FF) == 0xDC00) {
// Matching trailing surrogate.
result.add(onNonMatch(receiver.substring(i, i + 2)));
i += 2;
continue;
}
}
result.add(onNonMatch(receiver[i]));
i++;
}
result.add(onMatch(StringMatch(i, receiver, "")));
result.add(onNonMatch(""));
return result;
}

Iterable<T> stringSplitJSRegExpMapUnchecked<T>(String receiver,
JSSyntaxRegExp pattern, T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int startIndex = 0;
for (Match match in pattern.allMatches(receiver)) {
result.add(onNonMatch(receiver.substring(startIndex, match.start)));
result.add(onMatch(match));
startIndex = match.end;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

Iterable<T> stringSplitGeneralMapUnchecked<T>(String receiver, Pattern pattern,
T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int startIndex = 0;
for (Match match in pattern.allMatches(receiver)) {
result.add(onNonMatch(receiver.substring(startIndex, match.start)));
result.add(onMatch(match));
startIndex = match.end;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identical to stringSplitJSRegExpMapUnchecked, so no need to special case for JSRegExp.

}

@notNull
String replaceFirst(
Pattern from,
Expand Down
1 change: 1 addition & 0 deletions sdk/lib/_internal/js_runtime/lib/interceptors.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import 'dart:_js_helper'
stringIndexOfStringUnchecked,
stringLastIndexOfUnchecked,
stringReplaceAllFuncUnchecked,
stringSplitMapUnchecked,
stringReplaceAllUnchecked,
stringReplaceFirstUnchecked,
stringReplaceFirstMappedUnchecked,
Expand Down
93 changes: 93 additions & 0 deletions sdk/lib/_internal/js_runtime/lib/js_string.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,106 @@ final class JSString extends Interceptor
return stringReplaceAllFuncUnchecked(this, from, onMatch, onNonMatch);
}

Iterable<T> splitMap<T>(Pattern pattern,
{T Function(Match match)? onMatch, T Function(String nonMatch)? onNonMatch}) {
return stringSplitMapUnchecked(this, pattern, onMatch, onNonMatch);
}

String replaceFirst(Pattern from, String to, [int startIndex = 0]) {
checkString(to);
checkInt(startIndex);
RangeError.checkValueInInterval(startIndex, 0, this.length, "startIndex");
return stringReplaceFirstUnchecked(this, from, to, startIndex);
}

Iterable<T> stringSplitMapUnchecked<T>(String receiver, Pattern pattern,
T Function(Match)? onMatch, T Function(String)? onNonMatch) {
onMatch ??= (match) => match[0]! as T;
onNonMatch ??= (string) => string as T;
if (pattern is String) {
return stringSplitStringMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}
if (pattern is JSSyntaxRegExp) {
return stringSplitJSRegExpMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}
return stringSplitGeneralMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}

Iterable<T> stringSplitStringMapUnchecked<T>(String receiver, String pattern,
T Function(Match) onMatch, T Function(String) onNonMatch) {
if (pattern.isEmpty) {
return stringSplitEmptyMapUnchecked(receiver, onMatch, onNonMatch);
}
List<T> result = <T>[];
int startIndex = 0;
int patternLength = pattern.length;
while (true) {
int position = stringIndexOfStringUnchecked(receiver, pattern, startIndex);
if (position == -1) {
break;
}
result.add(onNonMatch(receiver.substring(startIndex, position)));
result.add(onMatch(StringMatch(position, receiver, pattern)));
startIndex = position + patternLength;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

Iterable<T> stringSplitEmptyMapUnchecked<T>(
String receiver, T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int length = receiver.length;
int i = 0;
result.add(onNonMatch(""));
while (i < length) {
result.add(onMatch(StringMatch(i, receiver, "")));
// Special case to avoid splitting a surrogate pair.
int code = receiver.codeUnitAt(i);
if ((code & ~0x3FF) == 0xD800 && length > i + 1) {
// Leading surrogate;
code = receiver.codeUnitAt(i + 1);
if ((code & ~0x3FF) == 0xDC00) {
// Matching trailing surrogate.
result.add(onNonMatch(receiver.substring(i, i + 2)));
i += 2;
continue;
}
}
result.add(onNonMatch(receiver[i]));
i++;
}
result.add(onMatch(StringMatch(i, receiver, "")));
result.add(onNonMatch(""));
return result;
}

Iterable<T> stringSplitJSRegExpMapUnchecked<T>(String receiver,
JSSyntaxRegExp pattern, T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int startIndex = 0;
for (Match match in pattern.allMatches(receiver)) {
result.add(onNonMatch(receiver.substring(startIndex, match.start)));
result.add(onMatch(match));
startIndex = match.end;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

Iterable<T> stringSplitGeneralMapUnchecked<T>(String receiver, Pattern pattern,
T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int startIndex = 0;
for (Match match in pattern.allMatches(receiver)) {
result.add(onNonMatch(receiver.substring(startIndex, match.start)));
result.add(onMatch(match));
startIndex = match.end;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

String replaceFirstMapped(Pattern from, String replace(Match match),
[int startIndex = 0]) {
checkNull(replace);
Expand Down
88 changes: 88 additions & 0 deletions sdk/lib/_internal/js_runtime/lib/string_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,94 @@ String stringReplaceAllFuncUnchecked(String receiver, Pattern pattern,
return buffer.toString();
}

Iterable<T> stringSplitMapUnchecked<T>(String receiver, Pattern pattern,
T Function(Match)? onMatch, T Function(String)? onNonMatch) {
onMatch ??= (match) => match[0]! as T;
onNonMatch ??= (string) => string as T;
if (pattern is String) {
return stringSplitStringMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}
if (pattern is JSSyntaxRegExp) {
return stringSplitJSRegExpMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}
return stringSplitGeneralMapUnchecked(receiver, pattern, onMatch, onNonMatch);
}

Iterable<T> stringSplitStringMapUnchecked<T>(String receiver, String pattern,
T Function(Match) onMatch, T Function(String) onNonMatch) {
if (pattern.isEmpty) {
return stringSplitEmptyMapUnchecked(receiver, onMatch, onNonMatch);
}
List<T> result = <T>[];
int startIndex = 0;
int patternLength = pattern.length;
while (true) {
int position = stringIndexOfStringUnchecked(receiver, pattern, startIndex);
if (position == -1) {
break;
}
result.add(onNonMatch(receiver.substring(startIndex, position)));
result.add(onMatch(StringMatch(position, receiver, pattern)));
startIndex = position + patternLength;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

Iterable<T> stringSplitEmptyMapUnchecked<T>(
String receiver, T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int length = receiver.length;
int i = 0;
result.add(onNonMatch(""));
while (i < length) {
result.add(onMatch(StringMatch(i, receiver, "")));
// Special case to avoid splitting a surrogate pair.
int code = receiver.codeUnitAt(i);
if ((code & ~0x3FF) == 0xD800 && length > i + 1) {
// Leading surrogate;
code = receiver.codeUnitAt(i + 1);
if ((code & ~0x3FF) == 0xDC00) {
// Matching trailing surrogate.
result.add(onNonMatch(receiver.substring(i, i + 2)));
i += 2;
continue;
}
}
result.add(onNonMatch(receiver[i]));
i++;
}
result.add(onMatch(StringMatch(i, receiver, "")));
result.add(onNonMatch(""));
return result;
}

Iterable<T> stringSplitJSRegExpMapUnchecked<T>(String receiver,
JSSyntaxRegExp pattern, T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int startIndex = 0;
for (Match match in pattern.allMatches(receiver)) {
result.add(onNonMatch(receiver.substring(startIndex, match.start)));
result.add(onMatch(match));
startIndex = match.end;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

Iterable<T> stringSplitGeneralMapUnchecked<T>(String receiver, Pattern pattern,
T Function(Match) onMatch, T Function(String) onNonMatch) {
List<T> result = <T>[];
int startIndex = 0;
for (Match match in pattern.allMatches(receiver)) {
result.add(onNonMatch(receiver.substring(startIndex, match.start)));
result.add(onMatch(match));
startIndex = match.end;
}
result.add(onNonMatch(receiver.substring(startIndex)));
return result;
}

String stringReplaceAllEmptyFuncUnchecked(String receiver,
String Function(Match) onMatch, String Function(String) onNonMatch) {
// Pattern is the empty string.
Expand Down
Loading