Skip to content

proposal: no_nullable_values_in_interpolated_strings #58615

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

Open
pq opened this issue Jan 12, 2022 · 8 comments
Open

proposal: no_nullable_values_in_interpolated_strings #58615

pq opened this issue Jan 12, 2022 · 8 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@pq
Copy link
Member

pq commented Jan 12, 2022

no_nullable_values_in_interpolated_strings

Description

Do not use nullable values in interpolated strings.

Details

TODO:

See: dart-lang/language#2053.

Kind

Error

Good Examples

void main() {
  String test = 'hi';
  String anotherTest = '$test';
  print(anotherTest);
}
main() {
  String? str = 'there';
  print('hi $str');
}
String? lastUpdatedTime;

Text('the last updated time is ${lastUpdatedTime!}');

Bad Examples

void main() {
  String? test;
  String anotherTest = '$test';
  print(anotherTest); 
}
String? lastUpdatedTime;

Text('the last updated time is $lastUpdatedTime');

Discussion

See: dart-lang/language#2053

@lrhn
Copy link
Member

lrhn commented Jan 12, 2022

The examples here suggests that the underlying rules would be:

  • Do not use a nullable expression in an interpolation (whether with or without braces),
  • unless the string literal containing the interpolation is the argument of print.

Correct?

@Levi-Lesches
Copy link

Levi-Lesches commented Jan 14, 2022

Given the discussion at dart-lang/language#2053, I'd say having a string that possibly contains the word null being printed on-screen should still be linted. Additionally, via any way strings can make it to the user, which infamously includes Text widgets. Given that, I'd say the lint should be at the string level itself. In fact, given that strings are almost always passed to humans (or other APIs), it should always be a bad sign when a nullable value makes its way into a string, since that can result in "null".

So, you'd end up with:

String? nullable;
String nonNull = "Hello, World!";

final a = "Welcome, $nonNull";			// ok, non-null value
final b = "Welcome, $nullable";			// lint: nullable expression in string interpolation
final c = "Welcome, ${nullable ?? 'user'}";	// ok, non-null value
final d = "Welcome, ${nullable!}";		// ok, assumed to be non-null
final e = "name=$?nullable";			// ok, value is allowed to be null, see dart-lang/language#2053
final f = "$a $b"				// ok, b is unsafe, but using its value is fine
print(b);					// ok, b is unsafe, but using its value is fine

@lrhn
Copy link
Member

lrhn commented Jan 14, 2022

The problem is that print("something $var1 $var2"); is a classical debugging thing to do, and it would get really annoying if that couldn't print null values.

In general, print is mainly intended for debugging. If you are writing a console application, you can, and probably should, use stdout.write instead. If you are on the web, a print is definitely debug information.

If we disable this lint only for string literals that are the argument expression of a print call, but don't try to do flow analysis to see where another string ends up, then I think we'll make debug-printing people happy, and won't affect much of anything else.

The alternative is to do flow analysis on the values (which is definitely approximative at best) and consider any string containing a nullable interpolation as "tainted". If it flows anywhere except into print or into another string interpolation (which will then also be tainted), then we lint.
I don't think that extra complication is actually worth it. All my debug-prints are string literals (I also don't care about 80 character line length in debug code, so I won't be introducing variables for sub-strings unless absolutely necessary).

If we actually introduce a "$?x" null-aware interpolation syntax into the language, I think we'd make normal interpolations non-nullable at the same time, so no lint would be necessary.

I'd be fine with only allowing "${?nullable}", not "$?nullable".
(We'd obviously have to decide what the interpolation puts into the string if the value is null, which will most likely be nothing (empty string, so ?nullable is equivalent to nullable ?? "".)

@AlexanderFarkas
Copy link

AlexanderFarkas commented Jan 14, 2022

What if I want to print such values for logging purposes? Either using a package called logger or debugLog. Restricting null acceptance only to print doesn't seem to be either useful or extensible.

@AlexanderFarkas
Copy link

@lrhn I suppose "${?nullable}" should print null, because putting null-awareness mark means, in almost all cases, that this value is used for debugging purposes.

If user deliberately refuses interpolation-soundness, they should process 'null-case' themselves (or leave it as is).

@srawlins srawlins added the P3 A lower priority bug or feature request label Oct 4, 2022
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 27, 2024
@srawlins
Copy link
Member

I was bit by this recently in dartdoc. Very surprising to see "foo/bar/null" show up as a URI. And hard to debug.

@Levi-Lesches
Copy link

For what it's worth regarding the earlier concerns about debug printing, if this is kept as a lint rather than a warning, it will be at the same level as avoid_print and therefore shouldn't bother too many people who just want a quick-and-dirty print statement and were okay with ignoring lints in the first place. (I'm also not attached to the $? syntax, I only brought it up from the previous discussion I linked).

@lrhn
Copy link
Member

lrhn commented Jun 18, 2024

Instead of being clever, we could also just say "no nullable values in interpolations" and require you to do "print("something ${var1.toString()} ${var2.toString()}"); if you want the "null"s printed.
It's a lint. You are asking for it. (But that may make it ineligible as a recommended lint.)

The $?var1 probably won't happen. I do hope we can make interpolations into "elements" like collection elements,
so you can do things like: print("something ${?var1} ${if (var1 != null) var1}, ${for (var x in [?var1, ?var2]) x}");.

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
@pq pq removed their assignment Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants