Skip to content

Commit 85227c4

Browse files
bkonyiCommit Queue
authored andcommitted
[ Analyzer ] Add diagnostics for Flutter Widget Previews
The Flutter Widget Previewer relies on code generation to import code from a developer's project into a generated 'widget preview scaffold' that lives in the project's '.dart_tool' directory. The nature of this implementation means that any symbols referenced in the declaration of the preview (e.g., in the invocation of the `Preview(...)` constructor or the name of the preview function) must be publicly accessible from outside the library in which they are defined. One of the main uses for this diagnostic is to flag usages of private symbols within preview declarations. These diagnostics also ensures that the `@Preview(...)` annotation can only be applied to a functions or constructors that: - Are statically accessible (e.g., no instance methods) - Have explicit implementations (e.g., not abstract or external) Fixes flutter/flutter#167193 Change-Id: I208c7f18c8d4ad826fed46a0dc625d090ff186ef Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423860 Commit-Queue: Ben Konyi <[email protected]> Reviewed-by: Paul Berry <[email protected]> Auto-Submit: Ben Konyi <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
1 parent f7ff419 commit 85227c4

File tree

13 files changed

+1143
-7
lines changed

13 files changed

+1143
-7
lines changed

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3635,6 +3635,10 @@ WarningCode.INVALID_USE_OF_VISIBLE_FOR_TEMPLATE_MEMBER:
36353635
status: noFix
36363636
WarningCode.INVALID_USE_OF_VISIBLE_FOR_TESTING_MEMBER:
36373637
status: noFix
3638+
WarningCode.INVALID_WIDGET_PREVIEW_APPLICATION:
3639+
status: needsEvaluation
3640+
WarningCode.INVALID_WIDGET_PREVIEW_PRIVATE_ARGUMENT:
3641+
status: needsEvaluation
36383642
WarningCode.INVALID_VISIBILITY_ANNOTATION:
36393643
status: hasFix
36403644
WarningCode.INVALID_VISIBLE_FOR_OVERRIDING_ANNOTATION:

pkg/analyzer/lib/src/error/best_practices_verifier.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import 'package:analyzer/src/error/doc_comment_verifier.dart';
3333
import 'package:analyzer/src/error/error_handler_verifier.dart';
3434
import 'package:analyzer/src/error/must_call_super_verifier.dart';
3535
import 'package:analyzer/src/error/null_safe_api_verifier.dart';
36+
import 'package:analyzer/src/error/widget_preview_verifier.dart';
3637
import 'package:analyzer/src/lint/constants.dart';
3738
import 'package:analyzer/src/utilities/extensions/ast.dart';
3839
import 'package:analyzer/src/utilities/extensions/element.dart';
@@ -81,6 +82,8 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
8182
_errorReporter,
8283
);
8384

85+
final WidgetPreviewVerifier _widgetPreviewVerifier;
86+
8487
/// The [WorkspacePackage] in which [_currentLibrary] is declared.
8588
final WorkspacePackage? _workspacePackage;
8689

@@ -129,6 +132,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
129132
),
130133
_mustCallSuperVerifier = MustCallSuperVerifier(_errorReporter),
131134
_nullSafeApiVerifier = NullSafeApiVerifier(_errorReporter, typeSystem),
135+
_widgetPreviewVerifier = WidgetPreviewVerifier(_errorReporter),
132136
_workspacePackage = workspacePackage {
133137
_deprecatedVerifier.pushInDeprecatedValue(_currentLibrary.hasDeprecated);
134138
_inDoNotStoreMember = _currentLibrary.metadata2.hasDoNotStore;
@@ -137,6 +141,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
137141
@override
138142
void visitAnnotation(Annotation node) {
139143
_annotationVerifier.checkAnnotation(node);
144+
_widgetPreviewVerifier.checkAnnotation(node);
140145
super.visitAnnotation(node);
141146
}
142147

@@ -1689,6 +1694,7 @@ class _InvalidAccessVerifier {
16891694
if (element == null) {
16901695
return;
16911696
}
1697+
16921698
_checkForInvalidDoNotSubmitAccess(identifier, element);
16931699

16941700
if (_inCurrentLibrary(element)) {

pkg/analyzer/lib/src/error/codes.g.dart

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6965,6 +6965,23 @@ class WarningCode extends ErrorCode {
69656965
hasPublishedDocs: true,
69666966
);
69676967

6968+
static const WarningCode INVALID_WIDGET_PREVIEW_APPLICATION = WarningCode(
6969+
'INVALID_WIDGET_PREVIEW_APPLICATION',
6970+
"The '@Preview(...)' annotation can only be applied to public, statically "
6971+
"accessible constructors and functions.",
6972+
);
6973+
6974+
/// Parameters:
6975+
/// 0: the name of the private symbol
6976+
/// 1: the name of the proposed public symbol equivalent
6977+
static const WarningCode
6978+
INVALID_WIDGET_PREVIEW_PRIVATE_ARGUMENT = WarningCode(
6979+
'INVALID_WIDGET_PREVIEW_PRIVATE_ARGUMENT',
6980+
"'@Preview(...)' can only accept arguments that consist of literals and "
6981+
"public symbols.",
6982+
correctionMessage: "Rename private symbol '{0}' to '{1}'.",
6983+
);
6984+
69686985
/// Parameters:
69696986
/// 0: the name of the member
69706987
static const WarningCode MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE =

pkg/analyzer/lib/src/error/error_code_values.g.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,8 @@ const List<ErrorCode> errorCodeValues = [
10461046
WarningCode.INVALID_VISIBILITY_ANNOTATION,
10471047
WarningCode.INVALID_VISIBLE_FOR_OVERRIDING_ANNOTATION,
10481048
WarningCode.INVALID_VISIBLE_OUTSIDE_TEMPLATE_ANNOTATION,
1049+
WarningCode.INVALID_WIDGET_PREVIEW_APPLICATION,
1050+
WarningCode.INVALID_WIDGET_PREVIEW_PRIVATE_ARGUMENT,
10491051
WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_ONE,
10501052
WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_THREE_PLUS,
10511053
WarningCode.MISSING_OVERRIDE_OF_MUST_BE_OVERRIDDEN_TWO,
Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/dart/ast/token.dart';
6+
import 'package:analyzer/dart/ast/visitor.dart';
7+
import 'package:analyzer/dart/element/element2.dart';
8+
import 'package:analyzer/error/listener.dart';
9+
import 'package:analyzer/src/dart/ast/ast.dart';
10+
import 'package:analyzer/src/error/codes.dart';
11+
import 'package:analyzer/src/utilities/extensions/flutter.dart';
12+
13+
/// Helper for verifying the validity of @Preview(...) applications.
14+
///
15+
/// The Flutter Widget Previewer relies on code generation to import code from
16+
/// a developer's project into a generated 'widget preview scaffold' that lives
17+
/// in the project's '.dart_tool' directory. The nature of this implementation
18+
/// means that any symbols referenced in the declaration of the preview (e.g.,
19+
/// in the invocation of the `Preview(...)` constructor or the name of the
20+
/// preview function) must be publicly accessible from outside the library in
21+
/// which they are defined. One of the main uses for this verifier is to flag
22+
/// usages of private symbols within preview declarations.
23+
///
24+
/// This verifier also ensures that the `@Preview(...)` annotation can only be
25+
/// applied to a functions or constructors that:
26+
///
27+
/// - Are statically accessible (e.g., no instance methods)
28+
/// - Have explicit implementations (e.g., not abstract or external)
29+
class WidgetPreviewVerifier {
30+
final ErrorReporter _errorReporter;
31+
32+
WidgetPreviewVerifier(this._errorReporter);
33+
34+
/// Check is [node] is a Widget Preview application and verify its
35+
/// correctness.
36+
void checkAnnotation(Annotation node) {
37+
if (node.elementAnnotation?.isWidgetPreview ?? false) {
38+
_checkWidgetPreview(node);
39+
}
40+
}
41+
42+
void _checkWidgetPreview(Annotation node) {
43+
if (node.arguments == null) {
44+
// This is an invalid annotation application since there's no constructor
45+
// invocation.
46+
return;
47+
}
48+
49+
var parent = node.parent;
50+
bool isValidApplication = switch (parent) {
51+
// First, check that the preview application is happening in a supported
52+
// context.
53+
_ when !_isSupportedParent(node: parent) => false,
54+
ConstructorDeclaration() => _isValidConstructorPreviewApplication(
55+
declaration: parent,
56+
),
57+
FunctionDeclaration() => _isValidFunctionPreviewApplication(
58+
declaration: parent,
59+
),
60+
MethodDeclaration() => _isValidMethodPreviewApplication(
61+
declaration: parent,
62+
),
63+
_ => false,
64+
};
65+
66+
if (!isValidApplication) {
67+
_errorReporter.atNode(
68+
node.name,
69+
WarningCode.INVALID_WIDGET_PREVIEW_APPLICATION,
70+
);
71+
}
72+
73+
var visitor = _InvalidWidgetPreviewArgumentDetectorVisitor(
74+
errorReporter: _errorReporter,
75+
);
76+
node.arguments!.accept(visitor);
77+
}
78+
79+
bool _hasRequiredParameters(NodeList<FormalParameter> parameters) {
80+
return parameters.any((e) => e.isRequired);
81+
}
82+
83+
/// Returns true if `name` is private or `node.parent` is a [ClassDeclaration]
84+
/// that has a private name.
85+
bool _isPrivateContext({required Token? name, required AstNode node}) {
86+
if (Identifier.isPrivateName(name?.lexeme ?? '')) {
87+
return true;
88+
}
89+
var parent = node.parent;
90+
if (parent == null) return false;
91+
92+
return switch (parent) {
93+
ClassDeclaration(:var name) => Identifier.isPrivateName(name.lexeme),
94+
EnumDeclaration(:var name) => Identifier.isPrivateName(name.lexeme),
95+
ExtensionDeclaration(:var name) => Identifier.isPrivateName(
96+
name?.lexeme ?? '',
97+
),
98+
ExtensionTypeDeclaration(:var name) => Identifier.isPrivateName(
99+
name.lexeme,
100+
),
101+
MixinDeclaration(:var name) => Identifier.isPrivateName(name.lexeme),
102+
_ => false,
103+
};
104+
}
105+
106+
/// Returns true if `node.parent` is a supported context for defining widget
107+
/// previews.
108+
///
109+
/// Currently, this only includes previews defined within classes and at the
110+
/// top level of a compilation unit.
111+
bool _isSupportedParent({required AstNode node}) {
112+
return switch (node.parent) {
113+
ClassDeclaration() || CompilationUnit() => true,
114+
_ => false,
115+
};
116+
}
117+
118+
/// Returns true if `declaration` is a valid constructor target for a widget
119+
/// preview application.
120+
///
121+
/// Constructor preview applications are valid if:
122+
/// - The class and constructor names are public
123+
/// - The class is a subtype of Widget
124+
/// - The class is not abstract or is a valid factory constructor
125+
/// - The constructor is not external
126+
/// - The constructor does not have any required arguments
127+
bool _isValidConstructorPreviewApplication({
128+
required ConstructorDeclaration declaration,
129+
}) {
130+
if (declaration case ConstructorDeclaration(
131+
:var name,
132+
:var externalKeyword,
133+
:var factoryKeyword,
134+
parent: ClassDeclaration(declaredFragment: ClassFragment(:var element)),
135+
parameters: FormalParameterList(:var parameters),
136+
)) {
137+
return !_isPrivateContext(name: name, node: declaration) &&
138+
element.isWidget &&
139+
!(element.isAbstract && factoryKeyword == null) &&
140+
externalKeyword == null &&
141+
!_hasRequiredParameters(parameters);
142+
}
143+
return false;
144+
}
145+
146+
/// Returns true if `declaration` is a valid top-level function target for a
147+
/// widget preview application.
148+
///
149+
/// Function preview applications are valid if:
150+
/// - The function name is public
151+
/// - The function is not a nested function
152+
/// - The function is not external
153+
/// - The function returns a subtype of `Widget` or `WidgetBuilder`
154+
/// - The function does not have any required arguments
155+
bool _isValidFunctionPreviewApplication({
156+
required FunctionDeclaration declaration,
157+
}) {
158+
if (declaration case FunctionDeclaration(
159+
:var name,
160+
:var externalKeyword,
161+
:NamedType returnType,
162+
functionExpression: FunctionExpression(
163+
parameters: FormalParameterList(:var parameters),
164+
),
165+
)) {
166+
return !_isPrivateContext(name: name, node: declaration) &&
167+
// Check for nested function.
168+
declaration.parent is! FunctionDeclarationStatement &&
169+
externalKeyword == null &&
170+
returnType.isValidWidgetPreviewReturnType &&
171+
!_hasRequiredParameters(parameters);
172+
}
173+
return false;
174+
}
175+
176+
/// Returns true if `declaration` is a valid static class member target for a
177+
/// widget preview application.
178+
///
179+
/// Class member preview applications are valid if:
180+
/// - The function is static
181+
/// - The function name is public
182+
/// - The function is not external
183+
/// - The function returns a subtype of `Widget` or `WidgetBuilder`
184+
/// - The function does not have any required arguments
185+
bool _isValidMethodPreviewApplication({
186+
required MethodDeclaration declaration,
187+
}) {
188+
if (declaration case MethodDeclaration(
189+
:var isStatic,
190+
:var externalKeyword,
191+
:var name,
192+
:NamedType returnType,
193+
parameters: FormalParameterList(:var parameters),
194+
)) {
195+
return !_isPrivateContext(name: name, node: declaration) &&
196+
isStatic &&
197+
// Check for nested function.
198+
declaration.parent is! FunctionDeclarationStatement &&
199+
externalKeyword == null &&
200+
returnType.isValidWidgetPreviewReturnType &&
201+
!_hasRequiredParameters(parameters);
202+
}
203+
return false;
204+
}
205+
}
206+
207+
class _InvalidWidgetPreviewArgumentDetectorVisitor extends RecursiveAstVisitor {
208+
final ErrorReporter errorReporter;
209+
210+
NamedExpression? rootArgument;
211+
_InvalidWidgetPreviewArgumentDetectorVisitor({required this.errorReporter});
212+
213+
@override
214+
void visitArgumentList(ArgumentList node) {
215+
for (var argument in node.arguments) {
216+
// All arguments to Preview(...) are named.
217+
if (argument is NamedExpression) {
218+
rootArgument = argument;
219+
visitNamedExpression(argument);
220+
rootArgument = null;
221+
}
222+
}
223+
}
224+
225+
@override
226+
void visitNamedExpression(NamedExpression node) {
227+
super.visitNamedExpression(node);
228+
}
229+
230+
@override
231+
void visitSimpleIdentifier(SimpleIdentifier node) {
232+
if (Identifier.isPrivateName(node.name)) {
233+
errorReporter.atNode(
234+
rootArgument!,
235+
WarningCode.INVALID_WIDGET_PREVIEW_PRIVATE_ARGUMENT,
236+
arguments: [node.name, node.name.replaceFirst(RegExp('_*'), '')],
237+
);
238+
}
239+
super.visitSimpleIdentifier(node);
240+
}
241+
}

0 commit comments

Comments
 (0)