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

Fix null safety code generation and migrate to null safety #222

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ analyzer:
exclude:
- lib/**/*.g.dart
- test/**/*.g.dart
errors:
uri_has_not_been_generated: ignore

linter:
rules:
Expand Down
4 changes: 1 addition & 3 deletions lib/src/generators/class_checks_collector.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @dart = 2.9

// Copyright 2017 Google Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -22,7 +20,7 @@ import 'methods/core.dart' as core;
String _generateCheck(Annotation annotation) {
if (annotation.arguments == null) throw 'bad';
return '${annotation.name.toString()}'
'(${annotation.arguments.arguments.join(', ')})';
'(${annotation.arguments?.arguments.join(', ')})';
}

/// Generates a comma separated list of class check declarations for the given
Expand Down
14 changes: 6 additions & 8 deletions lib/src/generators/collector_visitor.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @dart = 2.9

// Copyright 2017 Google Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -17,7 +15,7 @@ import 'dart:convert';

import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:pageloader/src/api/page_object_annotation.dart';
import '../api/page_object_annotation.dart';

import 'methods/core.dart';
import 'methods/core_method_information.dart';
Expand Down Expand Up @@ -305,12 +303,12 @@ class CollectorVisitor extends GeneralizingAstVisitor<void> {
: parameter;
final type =
(declaration is SimpleFormalParameter && declaration.type != null)
? declaration.type.toSource()
? declaration.type?.toSource()
: 'var';

var defaultValue = (parameter is DefaultFormalParameter &&
parameter.defaultValue != null)
? parameter.defaultValue.toSource()
? parameter.defaultValue?.toSource()
: null;

if (defaultValue == 'null') {
Expand All @@ -323,20 +321,20 @@ class CollectorVisitor extends GeneralizingAstVisitor<void> {

if (parameter.isRequired) {
buffer.writeln('''{
'name': '${parameter.declaredElement.name}',
'name': '${parameter.declaredElement?.name}',
'kind': 'required',
'type': '$type'
},''');
} else if (parameter.isNamed) {
buffer.writeln('''{
'name': '${parameter.declaredElement.name}',
'name': '${parameter.declaredElement?.name}',
'kind': 'named',
'type': '$type',
'default': $defaultValue
},''');
} else if (parameter.isOptionalPositional) {
buffer.writeln('''{
'name': '${parameter.declaredElement.name}',
'name': '${parameter.declaredElement?.name}',
'kind': 'positional',
'type': '$type',
'default': $defaultValue
Expand Down
15 changes: 7 additions & 8 deletions lib/src/generators/methods/annotation_evaluators.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @dart = 2.9

// Copyright 2017 Google Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -29,7 +27,8 @@ final String pageLoaderAnnotationInterface =

/// Returns set of AnnotationKinds that match '@root', '@Mouse', '@nullElement'
/// and '@Pointer'.
Set<AnnotationKind> evaluateAsAtomicAnnotation(Element element) {
/// TODO(null-safety): argument null-optional might be unnecessary
Set<AnnotationKind> evaluateAsAtomicAnnotation(Element? element) {
final returnSet = <AnnotationKind>{};
if (element is PropertyAccessorElement &&
element.library.name == pageLoaderAnnotations) {
Expand All @@ -43,18 +42,18 @@ Set<AnnotationKind> evaluateAsAtomicAnnotation(Element element) {

/// Returns set of AnnotationKinds if the annotation is a subclass of
/// Finder, Checker, and/or Filter.
Set<AnnotationKind> evaluateAsInterfaceAnnotation(Element element) {
final returnSet = <AnnotationKind>{};
Set<AnnotationKind?> evaluateAsInterfaceAnnotation(Element element) {
final returnSet = <AnnotationKind?>{};

DartType type;
DartType? type;
if (element is PropertyAccessorElement && element.isGetter) {
type = element.returnType;
} else if (element is ConstructorElement) {
type = element.returnType;
}

if (type is InterfaceType) {
final seenValidAnnotations = <AnnotationKind>{};
final seenValidAnnotations = <AnnotationKind?>{};
final interfaces = [type, ...type.allSupertypes];
for (var interface in interfaces) {
final interfaceElement = interface.element;
Expand Down Expand Up @@ -84,7 +83,7 @@ enum AnnotationKind {
}

/// Maps library and className to proper AnnotationKind.
AnnotationKind classNameToAnnotationKind(String className, {bool isAtomic}) {
AnnotationKind? classNameToAnnotationKind(String className, {required bool isAtomic}) {
if (isAtomic) {
switch (className) {
case 'LegacyPageObject':
Expand Down
29 changes: 15 additions & 14 deletions lib/src/generators/methods/core.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @dart = 2.9

// Copyright 2017 Google Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -40,12 +38,14 @@ final String pageObjectList = 'PageObjectList';

/// Returns a declaration of a annotation.
String generateAnnotationDeclaration(Annotation annotation) =>
'${annotation.name}(${annotation.arguments.arguments.join(", ")})';
'${annotation.name}(${annotation.arguments?.arguments.join(", ")})';

/// Returns a 'ByTagName' declaration from the 'ByCheckTag' annotation.
String generateByTagNameFromByCheckTag(
InterfaceType node, String methodSource) {
final defaultTagName = _extractTagName(node.element);
DartType? node, String methodSource) {
// Original(null-safety): node is InterfaceType and no "?"
// Also no cast below.
final defaultTagName = _extractTagName(node?.element as ClassElement);
if (defaultTagName.isEmpty) {
throw "'@ByCheckTag' can only be used on getters that return a "
"PageObject type with the'@CheckTag' annotation.\n\nCheck the type on "
Expand All @@ -58,15 +58,15 @@ String generateByTagNameFromByCheckTag(
/// If there is no tag name associated with the Page Object,
/// returns and empty string.
String _extractTagName(ClassElement poTypeElement) {
var expectedTag = '';
late String expectedTag;
for (final annotation in poTypeElement.metadata) {
final annotationElement = annotation.element;
if (annotationElement is ConstructorElement) {
final annotationName = annotationElement.enclosingElement.displayName;
final annotationValue = annotation.computeConstantValue();
if (annotationName == 'CheckTag') {
final inner = annotationValue.getField('_expectedTagName');
expectedTag = inner.toStringValue();
final inner = annotationValue?.getField('_expectedTagName');
expectedTag = inner?.toStringValue() ?? '';
}
}
}
Expand Down Expand Up @@ -100,8 +100,8 @@ bool isPageloaderAnnotation(Annotation annotation) =>
getAnnotationKind(annotation).isNotEmpty;

/// Returns set of all Pageloader annotation the current annotation satisfies.
Set<AnnotationKind> getAnnotationKind(Annotation annotation) {
final returnSet = <AnnotationKind>{};
Set<AnnotationKind?> getAnnotationKind(Annotation annotation) {
final returnSet = <AnnotationKind?>{};
final element = annotation.element;
if (element != null) {
returnSet
Expand Down Expand Up @@ -164,20 +164,21 @@ List<String> getReturnTypeArguments(String returnStr) {
/// matching name [matchingType].
///
/// Assumes that the inner-type has a single type (ex: not Foo<A, B>).
DartType getInnerType(DartType topType, String matchingType) {
DartType getInnerType(DartType? topType, String matchingType) {
// Filter out type name incase prefixes exist on the type.
matchingType =
matchingType.contains('.') ? matchingType.split('.')[1] : matchingType;
final typeArgs = (topType as ParameterizedType).typeArguments;
final first = typeArgs.first;
if (first.name == matchingType) {
// TODO(null-safety): nullability: true or false?
if (first.getDisplayString(withNullability: true) == matchingType) {
return first;
}
return getInnerType(first, matchingType);
}

/// Return the Dart code that corresponds to the [type].
String typeToCode(DartType type) {
String? typeToCode(DartType? type) {
// TODO: This should be replaced with actual code generation.
return type?.getDisplayString(withNullability: false);
return type?.getDisplayString(withNullability: true);
}
8 changes: 5 additions & 3 deletions lib/src/generators/methods/core_method_information.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @dart = 2.9

// Copyright 2017 Google Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -73,7 +71,11 @@ TypeInformation extractPageObjectInfo(

CoreMethodInformation collectCoreMethodInformation(MethodDeclaration node) {
// Extract type info.
final typeInfo = getTypeInformation(node.returnType.toSource());
// TODO(null-safety): verify logic.
// Before: final typeInfo = getTypeInformation((node.returnType).toSource());
// The reason is because for Dart, if a method doesn't declare a return type,
// it's automatically `dynamic`.
final typeInfo = getTypeInformation(node.returnType?.toSource() ?? 'dynamic');

final isFuture = typeInfo.type == 'Future';
final isList = typeInfo.type == 'List' ||
Expand Down
Loading