Conversation
Carriage returns, newlines, and unescaped single quotes, are all invalid characters in any single quoted string literal. Closes #2350 Expand the doc with a more explicit section describing that the resulting strings may included interpolation and that `raw` is the intended solution to write a literal based on the intended content instead of the intended source.
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with
Coverage
|
| File | Coverage |
|---|---|
| pkgs/code_builder/lib/src/specs/expression/literal.dart | 💔 Not covered |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
Unused Dependencies ⚠️
| Package | Status |
|---|---|
| code_builder | ❗ Show IssuesThese packages may be unused, or you may be using assets from these packages: |
For details on how to fix these, see dependency_validator.
This check can be disabled by tagging the PR with skip-unused-dependencies-check.
Breaking changes ✔️
| Package | Change | Current Version | New Version | Needed Version | Looking good? |
|---|---|---|---|---|---|
| code_builder | Non-Breaking | 4.11.1 | 4.11.2-wip | 4.11.2-wip | ✔️ |
This check can be disabled by tagging the PR with skip-breaking-check.
API leaks ✔️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
| Package | Leaked API symbol | Leaking sources |
|---|
This check can be disabled by tagging the PR with skip-leaking-check.
License Headers ✔️
// Copyright (c) 2026, 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.
| Files |
|---|
| no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/bazel_worker/benchmark/benchmark.dart |
| pkgs/coverage/lib/src/coverage_options.dart |
| pkgs/html/example/main.dart |
| pkgs/html/lib/dom.dart |
| pkgs/html/lib/dom_parsing.dart |
| pkgs/html/lib/html_escape.dart |
| pkgs/html/lib/parser.dart |
| pkgs/html/lib/src/constants.dart |
| pkgs/html/lib/src/encoding_parser.dart |
| pkgs/html/lib/src/html_input_stream.dart |
| pkgs/html/lib/src/list_proxy.dart |
| pkgs/html/lib/src/query_selector.dart |
| pkgs/html/lib/src/token.dart |
| pkgs/html/lib/src/tokenizer.dart |
| pkgs/html/lib/src/treebuilder.dart |
| pkgs/html/lib/src/utils.dart |
| pkgs/html/test/dom_test.dart |
| pkgs/html/test/parser_feature_test.dart |
| pkgs/html/test/parser_test.dart |
| pkgs/html/test/selectors/level1_baseline_test.dart |
| pkgs/html/test/selectors/level1_lib.dart |
| pkgs/html/test/selectors/selectors.dart |
| pkgs/html/test/support.dart |
| pkgs/html/test/tokenizer_test.dart |
| pkgs/html/test/trie_test.dart |
| pkgs/html/tool/generate_trie.dart |
| pkgs/pubspec_parse/test/git_uri_test.dart |
| pkgs/watcher/test/custom_watcher_factory_test.dart |
This check can be disabled by tagging the PR with skip-license-check.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the code_builder package to escape carriage return characters in the literalString function, alongside documentation improvements and a new test case. Feedback highlights a critical omission regarding backslash escaping, which could result in invalid Dart source code, and provides a specific code suggestion to resolve this.
Saying triple quoted strings are "unsupported" might make it seem like you can't pass it to this function which isn't the intended meaning.
The relationship is more clear between the argument _content_ and the result _source_.
We may want to deprecate the `raw` argument.
natebosch
changed the title
| return LiteralExpression._("${raw ? 'r' : ''}'$escaped'"); | ||
| } | ||
|
|
||
| String _escapeString(String value) { |
|
Some ongoing discussion about this API. Considering now whether it makes more sense to make a breaking change and use this escaping by default, with no option for the old behavior. I don't see any discussion about the intended use case when we added The only internal usage I can find doesn't appear to me to care about specifically allowing interpolation, and there are other code patterns possible to generate in place of interpolation. Folding down to the default safe behavior might be better UX. |
|
I'd rather have the breaking change, honestly. The fact that the current code allows bad output is really...bad. |
2 participants
Expand the doc with a more explicit description of the treatment of the
input and it's relationship to the source vs content of the result. Add
a
fullEscapeargument to replacerawwhich accomplishes nearly thesame, but restricts to input strings without single quotes. Document
this as the path to create a string literal with matching content
instead of matching source.
Add carriage returns to the set of escaped characters in non-fully
escaped single quoted strings. Other whitespace characters could be
escaped for clarity, but aren't needed for correctness. The full escape
path does escape other whitespace characters for clarity.
Closes #2350