Repository navigation
Getters don't show up consistently for inline values #5550
Description
import 'dart:developer';
var bar3 = 1;
class A {
String get bar1 => 'bar';
var bar2 = 1;
void foo() {
print(this.bar1);
print(this.bar2);
print(bar1);
print(bar2);
print(bar3);
var a = A();
print(a.bar1);
print(a.bar2);
print(A().bar1);
print(A().bar2);
debugger();
}
}
main() {
A().foo();
}
Activity
FMorschel commentedon Jun 16, 2025
I'm not sure we want to show getters. There shouldn't be side effects, but since they are getters, there can be.
Maybe @bwilkerson can weigh in.
DanTup commentedon Jun 16, 2025
Yeah, we only show getters when you have
dart.experimentalInlineValuesPropertiesenabled (and I'm still not entirely sure if we'll ultimately want to ship this on-by-default). But if we do/might, then we should handle all of the getters shown above, and not only the ones that are qualified likeinstance.getter).bwilkerson commentedon Jun 16, 2025
I agree that we want to be consistent and either show all getters or none of them.
I think we show getters in the debugger, so that argues in favor of showing them (although the debugger only displays getters for an object when you expand the object, so there's more user control). What are the reasons we might not want to?
DanTup commentedon Jun 16, 2025
My original idea was that we'd keep it behind a flag for a while to get feedback, then switch to using the same flag (which defaults to
true) as for showing getters in the debugger.However, I don't think these are completely equivalent for the reason you give - the debugger views are generally user-initiated (eg. you expand the parent object), so here there is an increased chance of triggering unexpected side-effects. I don't know if side effects in getters are common enough that it would be a real problem though. I suggest we continue to iterate (eg. fixing the bugs above) and then encourage more people to try out the flag and provide feedback.
bwilkerson commentedon Jun 16, 2025
That sounds good to me.
DanTup commentedon Feb 9, 2026
Looking at this again, I'm less sure now that we want to do this here. Yes, we do invoke getters when this setting is enabled, but as far as I can tell from my report here, the place where we're missing this is where there's also an invocation (a constructor in this case):
While we've accepted the side-effects of running getters, I'm less sure about invocations or constructors seems reasonable. Producing an instance of something to read the value for the InlayHint and then throwing it away feels odd, and if you had something like a generated
id, the one you see on the inlay hint wouldn't by the same as the one read in the code.So, my feeling is that we don't do this, and we draw the line at only ever invoking getters.
Of course, I'm interested in others opinions too.
FMorschel commentedon Feb 9, 2026
I agree. We could maybe enable this (for instantiation) only when cascades are used?
DanTup commentedon Feb 9, 2026
I'm not sure I understand why that's different?
FMorschel commentedon Feb 9, 2026
Because usually that is used to assign a variable.
And although doing the above is weird for production, it is a valid way to see the produced value (maybe there is some computation happening).
bwilkerson commentedon Feb 9, 2026
I agree.
Is the reasoning here that the result of
C()is cached (inv) so we could evaluatev.getterto get the value?If so, I don't think we want to perform the kind of analysis that would allow us to conclude that the value is cached in a variable in the plugin, and I don't think it would be worth the cost to add a protocol for this purpose to the analysis server.
DanTup commentedon Feb 9, 2026
LSP does already have support for this - we could return a range with a
String expressionon it that is different to the code in the range. We haven't implemented anything that does that so far though, we just support:However, I'm not convinced we should do this for a few reasons:
evaluateGettersInDebugViewsor similar) is an easily understandable linegetterspecifically has side-effects (invoking a stateless getter and throwing the value away seems pointless)? In which case, this kind of code is probably very uncommon (and perhaps a mistake)?DanTup commentedon Feb 11, 2026
Closing this for the reasons above, but happy to revisit if there are objections or other ideas.