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

Add --fix-typedefs to Dart formatter. #743

Merged
merged 8 commits into from Nov 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
169 changes: 131 additions & 38 deletions lib/src/source_visitor.dart
Expand Up @@ -112,6 +112,14 @@ class SourceVisitor extends ThrowingAstVisitor {
/// How many levels deep inside a constant context the visitor currently is.
int _constNesting = 0;

/// Whether we are currently fixing a typedef declaration.
///
/// Set to `true` while traversing the parameters of a typedef being converted
/// to the new syntax. The new syntax does not allow `int foo()` as a
/// parameter declaration, so it needs to be converted to `int Function() foo`
/// as part of the fix.
bool _insideNewTypedefFix = false;

/// A stack that tracks forcing nested collections to split.
///
/// Each entry corresponds to a collection currently being visited and the
Expand Down Expand Up @@ -1305,6 +1313,23 @@ class SourceVisitor extends ThrowingAstVisitor {
visitFunctionTypeAlias(FunctionTypeAlias node) {
visitMetadata(node.metadata);

if (_formatter.fixes.contains(StyleFix.typedefs)) {
_simpleStatement(node, () {
// Inlined visitGenericTypeAlias
lrhn marked this conversation as resolved.
Show resolved Hide resolved
_visitGenericTypeAliasHeader(node.typedefKeyword, node.name,
node.typeParameters, null, (node.returnType ?? node.name).offset);

space();

// Recursively convert function-arguments to Function syntax.
_insideNewTypedefFix = true;
_visitGenericFunctionType(
node.returnType, null, node.name.offset, null, node.parameters);
_insideNewTypedefFix = false;
});
return;
}

_simpleStatement(node, () {
token(node.typedefKeyword);
space();
Expand All @@ -1317,48 +1342,51 @@ class SourceVisitor extends ThrowingAstVisitor {

visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) {
visitParameterMetadata(node.metadata, () {
modifier(node.covariantKeyword);
visit(node.returnType, after: space);
if (!_insideNewTypedefFix) {
modifier(node.covariantKeyword);
visit(node.returnType, after: space);
// Try to keep the function's parameters with its name.
builder.startSpan();
visit(node.identifier);
_visitParameterSignature(node.typeParameters, node.parameters);
builder.endSpan();
} else {
// In-lined from visitSimpleFormalParameter.
builder.startLazyRule(new Rule(Cost.parameterType));
builder.nestExpression();
modifier(node.covariantKeyword);

// Try to keep the function's parameters with its name.
builder.startSpan();
visit(node.identifier);
_visitParameterSignature(node.typeParameters, node.parameters);
builder.endSpan();
});
}
// In-lined vistGenericFunctionType
builder.startLazyRule();
builder.nestExpression();

visitGenericFunctionType(GenericFunctionType node) {
builder.startLazyRule();
builder.nestExpression();
visit(node.returnType, after: split);
_writeText("Function", node.identifier.offset);

visit(node.returnType, after: split);
token(node.functionKeyword);
builder.unnest();
builder.endRule();
_visitParameterSignature(node.typeParameters, node.parameters);
// End of visitGenericFunctionType
split();

builder.unnest();
builder.endRule();
visit(node.identifier);
builder.unnest();
builder.endRule();
// End of visitSimpleFormalParameter
}
});
}

_visitParameterSignature(node.typeParameters, node.parameters);
visitGenericFunctionType(GenericFunctionType node) {
_visitGenericFunctionType(node.returnType, node.functionKeyword, null,
node.typeParameters, node.parameters);
}

visitGenericTypeAlias(GenericTypeAlias node) {
visitNodes(node.metadata, between: newline, after: newline);
_simpleStatement(node, () {
token(node.typedefKeyword);
space();

// If the typedef's type parameters split, split after the "=" too,
// mainly to ensure the function's type parameters and parameters get
// end up on successive lines with the same indentation.
builder.startRule();

visit(node.name);

visit(node.typeParameters);
split();

token(node.equals);
builder.endRule();
_visitGenericTypeAliasHeader(node.typedefKeyword, node.name,
node.typeParameters, node.equals, null);

space();

Expand Down Expand Up @@ -1835,14 +1863,24 @@ class SourceVisitor extends ThrowingAstVisitor {
modifier(node.covariantKeyword);
modifier(node.keyword);

visit(node.type);
bool hasType = node.type != null;
if (_insideNewTypedefFix && !hasType) {
// In function declarations and the old typedef syntax, you can have a
// parameter name without a type. In the new syntax, you can have a type
// without a name. Add "dynamic" in that case.

// Ensure comments on the identifier comes before the inserted type.
token(node.identifier.token, before: () {
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I am kind-of inlining visitSimpleIdentifier, which just calls token(node.token).
Should I instead add an {void before()} or {void beforeToken} parameter to visitSimpleIdentifier and call that directly?

I do want to insert the dynamic between the comment and the token itself, which token(..., before: ...) does for me, but I can't put that extra aargument through the visit/accept procedure, and I don't want to store it in some object-global state.

_writeText("dynamic", node.identifier.offset);
split();
});
} else {
visit(node.type);

// In function declarations and the old typedef syntax, you can have a
// parameter name without a type. In the new syntax, you can have a type
// without a name. Handle both cases.
if (node.type != null && node.identifier != null) split();
if (hasType && node.identifier != null) split();

visit(node.identifier);
visit(node.identifier);
}
builder.unnest();
builder.endRule();
});
Expand Down Expand Up @@ -2584,6 +2622,61 @@ class SourceVisitor extends ThrowingAstVisitor {
}
}

/// Writes a `Function` function type.
///
/// Used also by a fix, so there may not be a [functionKeyword].
/// In that case [functionKeywordPosition] should be the source position
/// used for the inserted "Function" text.
void _visitGenericFunctionType(
AstNode returnType,
Token functionKeyword,
int functionKeywordPosition,
TypeParameterList typeParameters,
FormalParameterList parameters) {
builder.startLazyRule();
builder.nestExpression();

visit(returnType, after: split);
if (functionKeyword != null) {
token(functionKeyword);
} else {
_writeText("Function", functionKeywordPosition);
}

builder.unnest();
builder.endRule();
_visitParameterSignature(typeParameters, parameters);
}

/// Writes the header of a new-style typedef.
///
/// Also used by a fix so there may not be an [equals] token.
/// If [equals] is `null`, then [equalsPosition] must be a
/// position to use for the inserted text "=".
void _visitGenericTypeAliasHeader(Token typedefKeyword, AstNode name,
AstNode typeParameters, Token equals, int equalsPosition) {
token(typedefKeyword);
space();

// If the typedef's type parameters split, split after the "=" too,
// mainly to ensure the function's type parameters and parameters get
// end up on successive lines with the same indentation.
builder.startRule();

visit(name);

visit(typeParameters);
split();

if (equals != null) {
token(equals);
} else {
_writeText("=", equalsPosition);
}

builder.endRule();
}

/// Gets the cost to split at an assignment (or `:` in the case of a named
/// default value) with the given [rightHandSide].
///
Expand Down
5 changes: 5 additions & 0 deletions lib/src/style_fix.dart
@@ -1,4 +1,5 @@
// Copyright (c) 2018, 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.

Expand All @@ -18,11 +19,15 @@ class StyleFix {
static const docComments = const StyleFix._(
"doc-comments", 'Use triple slash for documentation comments.');

static const typedefs =
const StyleFix._("typedefs", 'Use new typedef syntax for typedefs.');

static const all = const [
namedDefaultSeparator,
optionalConst,
optionalNew,
docComments,
typedefs,
];

final String name;
Expand Down
1 change: 1 addition & 0 deletions test/fix_test.dart
Expand Up @@ -17,4 +17,5 @@ void main() {
testFile("fixes/doc_comments.stmt", [StyleFix.docComments]);
testFile("fixes/optional_const.unit", [StyleFix.optionalConst]);
testFile("fixes/optional_new.stmt", [StyleFix.optionalNew]);
testFile("fixes/typedefs.unit", [StyleFix.typedefs]);
}