From 30378c99c179a9b37644ebff56a5bc6e6ca3f25d Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Wed, 6 Apr 2022 13:37:38 -0700 Subject: [PATCH] Sync Android AccessibilityFeature enum and dart:ui This brings the Android accessibility bridge AccessibilityFeature enum back in sync with the `AccessibilityFeatures` class in dart:ui, defined in lib/ui/window.dart Keeping these enums in sync aids in automated API consistency checking. Issue: https://github.com/flutter/flutter/issues/101217 --- testing/run_tests.py | 17 +++ tools/api_check/README.md | 18 +++ tools/api_check/lib/apicheck.dart | 158 ++++++++++++++++++++++++ tools/api_check/pubspec.yaml | 84 +++++++++++++ tools/api_check/test/apicheck_test.dart | 140 +++++++++++++++++++++ tools/pub_get_offline.py | 1 + 6 files changed, 418 insertions(+) create mode 100644 tools/api_check/README.md create mode 100644 tools/api_check/lib/apicheck.dart create mode 100644 tools/api_check/pubspec.yaml create mode 100644 tools/api_check/test/apicheck_test.dart diff --git a/testing/run_tests.py b/testing/run_tests.py index 9b88e14b1e848..305d730440586 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -544,6 +544,22 @@ def RunClangTidyTests(build_dir): cwd=test_dir) +def RunApiConsistencyTests(build_dir): + test_dir = os.path.join(buildroot_dir, 'flutter', 'tools', 'api_check') + dart_tests = glob.glob('%s/test/*_test.dart' % test_dir) + for dart_test_file in dart_tests: + opts = [ + '--disable-dart-dev', + dart_test_file, + os.path.join(buildroot_dir, 'flutter')] + RunEngineExecutable( + build_dir, + os.path.join('dart-sdk', 'bin', 'dart'), + None, + flags=opts, + cwd=test_dir) + + def main(): parser = argparse.ArgumentParser() all_types = ['engine', 'dart', 'benchmarks', 'java', 'android', 'objc', 'font-subset'] @@ -611,6 +627,7 @@ def main(): RunLitetestTests(build_dir) RunGithooksTests(build_dir) RunClangTidyTests(build_dir) + RunApiConsistencyTests(build_dir) RunDartTests(build_dir, dart_filter, args.verbose_dart_snapshot) RunConstFinderTests(build_dir) RunFrontEndServerTests(build_dir) diff --git a/tools/api_check/README.md b/tools/api_check/README.md new file mode 100644 index 0000000000000..a1bdfbe1f44ea --- /dev/null +++ b/tools/api_check/README.md @@ -0,0 +1,18 @@ +# API consistency check tool + +Verifies that enums in each of the platform-specific embedders, and the embedder +API remain consistent with their API in dart:ui. + +### Running the tool + +This tool is run as part of `testing/run_tests.sh`. + +To run the tool, invoke with the path of the Flutter engine repo as the first +argument. + +``` +../../../out/host_debug_unopt/dart-sdk/bin/dart \ + --disable-dart-dev \ + test/apicheck_test.dart \ + "$(dirname $(dirname $PWD))" +``` diff --git a/tools/api_check/lib/apicheck.dart b/tools/api_check/lib/apicheck.dart new file mode 100644 index 0000000000000..86ee51dd30a27 --- /dev/null +++ b/tools/api_check/lib/apicheck.dart @@ -0,0 +1,158 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:convert' show LineSplitter; +import 'dart:io'; + +import 'package:analyzer/dart/analysis/analysis_context_collection.dart'; +import 'package:analyzer/dart/analysis/analysis_context.dart'; +import 'package:analyzer/dart/analysis/results.dart'; +import 'package:analyzer/dart/analysis/session.dart'; +import 'package:analyzer/dart/ast/ast.dart'; + +/// Returns all indexed fields in [className]. +/// +/// Field names are expected to be of the form `kFooBarIndex`; prefixed with a +/// `k` and terminated in `Index`. +List getDartClassFields({ + required String sourcePath, + required String className, +}) { + final List includedPaths = [sourcePath]; + final AnalysisContextCollection collection = AnalysisContextCollection(includedPaths: includedPaths); + final AnalysisContext context = collection.contextFor(sourcePath); + final AnalysisSession session = context.currentSession; + + final result = session.getParsedUnit(sourcePath); + if (result is! ParsedUnitResult) { + return []; + } + + // Locate all fields matching the expression in the class. + final RegExp fieldExp = RegExp(r'_k(\w*)Index'); + final List fields = []; + for (CompilationUnitMember unitMember in result.unit.declarations) { + if (unitMember is ClassDeclaration && unitMember.name.name == className) { + for (ClassMember classMember in unitMember.members) { + if (classMember is FieldDeclaration) { + for (VariableDeclaration field in classMember.fields.variables) { + final String fieldName = field.name.name; + final RegExpMatch? match = fieldExp.firstMatch(fieldName); + if (match != null) { + fields.add(match.group(1)!); + } + } + } + } + } + } + return fields; +} + +/// Returns all values in [enumName]. +/// +/// Enum values are expected to be of the form `kEnumNameFooBar`; prefixed with +/// `kEnumName`. +List getCppEnumValues({ + required String sourcePath, + required String enumName, +}) { + List lines = File(sourcePath).readAsLinesSync(); + final int enumEnd = lines.indexOf('} $enumName;'); + if (enumEnd < 0) { + return []; + } + final int enumStart = lines.lastIndexOf('typedef enum {', enumEnd); + if (enumStart < 0 || enumStart >= enumEnd) { + return []; + } + final valueExp = RegExp('^\\s*k$enumName(\\w*)'); + return _extractMatchingExpression( + lines: lines.sublist(enumStart + 1, enumEnd), + regexp: valueExp, + ); +} + +/// Returns all values in [enumName]. +/// +/// Enum values are expected to be of the form `kFooBar`; prefixed with `k`. +List getCppEnumClassValues({ + required String sourcePath, + required String enumName, +}) { + final List lines = _getBlockStartingWith( + source: File(sourcePath).readAsStringSync(), + startExp: RegExp('enum class $enumName .* {'), + ); + final valueExp = RegExp(r'^\s*k(\w*)'); + return _extractMatchingExpression(lines: lines, regexp: valueExp); +} + +/// Returns all values in [enumName]. +/// +/// Enum value declarations are expected to be of the form `FOO_BAR(1 << N)`; +/// in all caps. +List getJavaEnumValues({ + required String sourcePath, + required String enumName, +}) { + final List lines = _getBlockStartingWith( + source: File(sourcePath).readAsStringSync(), + startExp: RegExp('enum $enumName {'), + ); + final RegExp valueExp = RegExp(r'^\s*([A-Z_]*)\('); + return _extractMatchingExpression(lines: lines, regexp: valueExp); +} + +/// Returns all values in [lines] whose line of code matches [regexp]. +/// +/// The contents of the first match group in [regexp] is returned; therefore +/// it must contain a match group. +List _extractMatchingExpression({ + required Iterable lines, + required RegExp regexp, +}) { + List values = []; + for (String line in lines) { + final RegExpMatch? match = regexp.firstMatch(line); + if (match != null) { + values.add(match.group(1)!); + } + } + return values; +} + +/// Returns all lines of the block starting with [startString]. +/// +/// [startString] MUST end with '{'. +List _getBlockStartingWith({ + required String source, + required RegExp startExp, +}) { + assert(startExp.pattern.endsWith('{')); + + final int blockStart = source.indexOf(startExp); + if (blockStart < 0) { + return []; + } + // Find start of block. + int pos = blockStart; + while (pos < source.length && source[pos] != '{') { + pos++; + } + int braceCount = 1; + + // Count braces until end of block. + pos++; + while (pos < source.length && braceCount > 0) { + if (source[pos] == '{') { + braceCount++; + } else if (source[pos] == '}') { + braceCount--; + } + pos++; + } + final int blockEnd = pos; + return LineSplitter.split(source, blockStart, blockEnd).toList(); +} diff --git a/tools/api_check/pubspec.yaml b/tools/api_check/pubspec.yaml new file mode 100644 index 0000000000000..ec85fb8529e03 --- /dev/null +++ b/tools/api_check/pubspec.yaml @@ -0,0 +1,84 @@ +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +name: apicheck +publish_to: none + +# Do not add any dependencies that require more than what is provided in +# //third_party/dart/pkg or //third_party/dart/third_party/pkg. +# In particular, package:test is not usable here. + +# If you do add packages here, make sure you can run `pub get --offline`, and +# check the .packages and .package_config to make sure all the paths are +# relative to this directory into //third_party/dart + +environment: + sdk: '>=2.12.0 <3.0.0' + +# Do not add any dependencies that require more than what is provided in +# //third_party/pkg, //third_party/dart/pkg, or +# //third_party/dart/third_party/pkg. In particular, package:test is not usable +# here. + +# If you do add packages here, make sure you can run `pub get --offline`, and +# check the .packages and .package_config to make sure all the paths are +# relative to this directory into //third_party/dart + +dependencies: + analyzer: any + _fe_analyzer_shared: any + +dev_dependencies: + async_helper: any + expect: any + litetest: any + smith: any + +dependency_overrides: + _fe_analyzer_shared: + path: ../../../third_party/dart/pkg/_fe_analyzer_shared + analyzer: + path: ../../../third_party/dart/pkg/analyzer + async: + path: ../../../third_party/dart/third_party/pkg/async + async_helper: + path: ../../../third_party/dart/pkg/async_helper + charcode: + path: ../../../third_party/dart/third_party/pkg/charcode + collection: + path: ../../../third_party/dart/third_party/pkg/collection + convert: + path: ../../../third_party/dart/third_party/pkg/convert + crypto: + path: ../../../third_party/dart/third_party/pkg/crypto + expect: + path: ../../../third_party/dart/pkg/expect + file: + path: ../../../third_party/pkg/file/packages/file + glob: + path: ../../../third_party/dart/third_party/pkg/glob + litetest: + path: ../../testing/litetest + meta: + path: ../../../third_party/dart/pkg/meta + package_config: + path: ../../../third_party/dart/third_party/pkg_tested/package_config + path: + path: ../../../third_party/dart/third_party/pkg/path + pub_semver: + path: ../../../third_party/dart/third_party/pkg/pub_semver + source_span: + path: ../../../third_party/dart/third_party/pkg/source_span + string_scanner: + path: ../../../third_party/dart/third_party/pkg/string_scanner + term_glyph: + path: ../../../third_party/dart/third_party/pkg/term_glyph + typed_data: + path: ../../../third_party/dart/third_party/pkg/typed_data + watcher: + path: ../../../third_party/dart/third_party/pkg/watcher + yaml: + path: ../../../third_party/dart/third_party/pkg/yaml + smith: + path: ../../../third_party/dart/pkg/smith diff --git a/tools/api_check/test/apicheck_test.dart b/tools/api_check/test/apicheck_test.dart new file mode 100644 index 0000000000000..b389f72e77d7a --- /dev/null +++ b/tools/api_check/test/apicheck_test.dart @@ -0,0 +1,140 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io'; + +import 'package:apicheck/apicheck.dart'; +import 'package:litetest/litetest.dart'; + +/// Verify that duplicate Flutter API is consistent between implementations. +/// +/// Flutter contains API that is required to match between implementations. +/// Notably, some enums such as those used by Flutter accessibility, appear in: +/// * dart:ui (native) +/// * dart:ui (web) +/// * embedder.h +/// * Internal enumerations used by iOS/Android +/// +/// WARNING: The embedder API makes strong API/ABI stability guarantees. Care +/// must be taken when adding new enums, or values to existing enums. These +/// CANNOT be removed without breaking backward compatibility, which we have +/// never done. See the note at the top of `shell/platform/embedder/embedder.h` +/// for further details. +main(List arguments) { + if (arguments.length < 1) { + print('usage: dart bin/apicheck.dart path/to/engine/src/flutter'); + exit(1); + } + final String flutterRoot = arguments[0]; + + test('AccessibilityFeatures enums match', () { + // Dart values: _kFooBarIndex = 1 << N + List uiFields = getDartClassFields( + sourcePath:'$flutterRoot/lib/ui/window.dart', + className:'AccessibilityFeatures', + ); + List webuiFields = getDartClassFields( + sourcePath:'$flutterRoot/lib/ui/window.dart', + className:'AccessibilityFeatures', + ); + // C values: kFlutterAccessibilityFeatureFooBar = 1 << N, + List embedderEnumValues = getCppEnumValues( + sourcePath: '$flutterRoot/shell/platform/embedder/embedder.h', + enumName: 'FlutterAccessibilityFeature', + ); + // C++ values: kFooBar = 1 << N, + List internalEnumValues = getCppEnumClassValues( + sourcePath: '$flutterRoot/lib/ui/window/platform_configuration.h', + enumName: 'AccessibilityFeatureFlag', + ); + // Java values: FOO_BAR(1 << N). + List javaEnumValues = getJavaEnumValues( + sourcePath: '$flutterRoot/shell/platform/android/io/flutter/view/AccessibilityBridge.java', + enumName: 'AccessibilityFeature', + ).map(allCapsToCamelCase).toList(); + + expect(webuiFields, uiFields); + expect(embedderEnumValues, uiFields); + expect(internalEnumValues, uiFields); + expect(javaEnumValues, uiFields); + }); + + test('SemanticsAction enums match', () { + // Dart values: _kFooBarIndex = 1 << N. + List uiFields = getDartClassFields( + sourcePath:'$flutterRoot/lib/ui/semantics.dart', + className:'SemanticsAction', + ); + List webuiFields = getDartClassFields( + sourcePath:'$flutterRoot/lib/ui/semantics.dart', + className:'SemanticsAction', + ); + // C values: kFlutterSemanticsActionFooBar = 1 << N. + List embedderEnumValues = getCppEnumValues( + sourcePath: '$flutterRoot/shell/platform/embedder/embedder.h', + enumName: 'FlutterSemanticsAction', + ); + // C++ values: kFooBar = 1 << N. + List internalEnumValues = getCppEnumClassValues( + sourcePath: '$flutterRoot/lib/ui/semantics/semantics_node.h', + enumName: 'SemanticsAction', + ); + // Java values: FOO_BAR(1 << N). + List javaEnumValues = getJavaEnumValues( + sourcePath: '$flutterRoot/shell/platform/android/io/flutter/view/AccessibilityBridge.java', + enumName: 'Action', + ).map(allCapsToCamelCase).toList(); + + expect(webuiFields, uiFields); + expect(embedderEnumValues, uiFields); + expect(internalEnumValues, uiFields); + expect(javaEnumValues, uiFields); + }); + + test('SemanticsFlag enums match', () { + // Dart values: _kFooBarIndex = 1 << N. + List uiFields = getDartClassFields( + sourcePath:'$flutterRoot/lib/ui/semantics.dart', + className:'SemanticsFlag', + ); + List webuiFields = getDartClassFields( + sourcePath:'$flutterRoot/lib/ui/semantics.dart', + className:'SemanticsFlag', + ); + // C values: kFlutterSemanticsFlagFooBar = 1 << N. + List embedderEnumValues = getCppEnumValues( + sourcePath: '$flutterRoot/shell/platform/embedder/embedder.h', + enumName: 'FlutterSemanticsFlag', + ); + // C++ values: kFooBar = 1 << N. + List internalEnumValues = getCppEnumClassValues( + sourcePath: '$flutterRoot/lib/ui/semantics/semantics_node.h', + enumName: 'SemanticsFlags', + ); + // Java values: FOO_BAR(1 << N). + List javaEnumValues = getJavaEnumValues( + sourcePath: '$flutterRoot/shell/platform/android/io/flutter/view/AccessibilityBridge.java', + enumName: 'Flag', + ).map(allCapsToCamelCase).toList(); + + expect(webuiFields, uiFields); + expect(embedderEnumValues, uiFields); + expect(internalEnumValues, uiFields); + expect(javaEnumValues, uiFields); + }); +} + +/// Returns the CamelCase equivalent of an ALL_CAPS identifier. +String allCapsToCamelCase(String identifier) { + StringBuffer buffer = StringBuffer(); + for (String word in identifier.split('_')) { + if (word.isNotEmpty) { + buffer.write(word[0]); + } + if (word.length > 1) { + buffer.write(word.substring(1).toLowerCase()); + } + } + return buffer.toString(); +} diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index c978e8ddac548..911a3cb8fa7d6 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -25,6 +25,7 @@ os.path.join("src", "flutter", "testing", "scenario_app"), os.path.join("src", "flutter", "testing", "smoke_test_failure"), os.path.join("src", "flutter", "testing", "symbols"), + os.path.join("src", "flutter", "tools", "api_check"), os.path.join("src", "flutter", "tools", "android_lint"), os.path.join("src", "flutter", "tools", "clang_tidy"), os.path.join("src", "flutter", "tools", "const_finder"),