From 18071ec0a87c281edffa0179228560dfe1f6f357 Mon Sep 17 00:00:00 2001 From: Younghan Kim Date: Mon, 11 Nov 2024 11:47:32 +0900 Subject: [PATCH] Remove block and line comments when detecting '.flutter-plugins' in settings.gradle(.kts) (#155488) **PR Title:** Remove block and line comments when detecting `'.flutter-plugins'` in `settings.gradle` --- **Description:** This PR modifies the `configureLegacyPluginEachProjects` function to remove block (`/* ... */`) and line (`// ...`) comments from the `settings.gradle` or `settings.gradle.kts` file content before checking for the presence of the `'.flutter-plugins'` string. This ensures that only uncommented, meaningful code is considered during the detection, preventing false positives when the string appears within comments. **Why is this change necessary?** In some cases, the `'.flutter-plugins'` string may be present inside comments in the `settings.gradle` file. The existing implementation does not account for this and may incorrectly detect the string even when it's commented out. This can lead to unintended behavior, such as configuring plugin projects when it is not necessary. By removing comments before performing the check, we prevent false positives and ensure that the detection logic is accurate, only acting when the `'.flutter-plugins'` string is present in active code. **Changes Made:** - **Added comment removal logic:** - Removed block comments (`/* ... */`) using the regular expression `/(?s)\/\*.*?\*\//`. - The `(?s)` flag enables dot-all mode, allowing `.` to match newline characters. - Removed line comments (`// ...`) using the regular expression `/(?m)\/\/.*$`. - The `(?m)` flag enables multi-line mode, so `^` and `$` match the start and end of each line. - Combined both comment removal steps into a single chain for efficiency. - **Updated the string detection:** - The check for `'.flutter-plugins'` is now performed on the uncommented content of the `settings.gradle` file. - This ensures that only meaningful, uncommented code is considered during detection. **Issue Fixed:** - Fixes [#155484](https://github.com/flutter/flutter/issues/155484) --- --- If you need any further assistance or have questions, feel free to reach out! --- **Links:** - [Contributor Guide] - [Tree Hygiene] - [Flutter Style Guide] - [Features we expect every widget to implement] - [CLA] - [flutter/tests] - [breaking change policy] - [Discord] - [Data Driven Fixes] --- .../gradle/src/main/groovy/flutter.groovy | 7 +- ...tter_plugins_strings_in_comments_test.dart | 98 +++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 packages/flutter_tools/test/integration.shard/android_gradle_legacy_flutter_plugins_strings_in_comments_test.dart diff --git a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy index 4949b597fc..f1661ec0da 100644 --- a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy +++ b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy @@ -677,7 +677,12 @@ class FlutterPlugin implements Plugin { */ private void configureLegacyPluginEachProjects(Project project) { try { - if (!settingsGradleFile(project).text.contains("'.flutter-plugins'")) { + // Read the contents of the settings.gradle file. + // Remove block/line comments + String settingsText = settingsGradleFile(project).text + settingsText = settingsText.replaceAll(/(?s)\/\*.*?\*\//, '').replaceAll(/(?m)\/\/.*$/, '') + + if (!settingsText.contains("'.flutter-plugins'")) { return } } catch (FileNotFoundException ignored) { diff --git a/packages/flutter_tools/test/integration.shard/android_gradle_legacy_flutter_plugins_strings_in_comments_test.dart b/packages/flutter_tools/test/integration.shard/android_gradle_legacy_flutter_plugins_strings_in_comments_test.dart new file mode 100644 index 0000000000..d5d7a46d7c --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/android_gradle_legacy_flutter_plugins_strings_in_comments_test.dart @@ -0,0 +1,98 @@ +// Copyright 2014 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. + +// This test can be removed once https://github.com/flutter/flutter/issues/155484 is resolved. + +import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/cache.dart'; + +import '../src/common.dart'; +import 'test_utils.dart'; + +void main() { + late Directory tempDir; + + setUp(() { + Cache.flutterRoot = getFlutterRoot(); + tempDir = createResolvedTempDirectorySync('flutter_plugin_test.'); + }); + + tearDown(() async { + tryToDelete(tempDir); + }); + + test('should build Android app with commented-out ".flutter-plugins" in settings.gradle', () async { + final String flutterBin = fileSystem.path.join( + getFlutterRoot(), + 'bin', + 'flutter', + ); + + // Create Android app project instead of plugin + processManager.runSync([ + flutterBin, + ...getLocalEngineArguments(), + 'create', + '--template=app', + 'test_android_app', + ], workingDirectory: tempDir.path); + + final Directory appDir = tempDir.childDirectory('test_android_app'); + final Directory androidDir = appDir.childDirectory('android'); + + // Create settings.gradle with commented .flutter-plugins + final File settingsGradle = androidDir.childFile('settings.gradle'); + settingsGradle.writeAsStringSync(r''' +// The following block uses '.flutter-plugins' but is commented out. +/* +def plugins = new Properties() +def pluginsFile = new File(flutterProjectRoot.toFile(), '.flutter-plugins') +if (pluginsFile.exists()) { + pluginsFile.withReader('UTF-8') { reader -> plugins.load(reader) } +} +*/ + +pluginManagement { + def flutterSdkPath = { + def properties = new Properties() + file("local.properties").withInputStream { properties.load(it) } + def flutterSdkPath = properties.getProperty("flutter.sdk") + assert flutterSdkPath != null, "flutter.sdk not set in local.properties" + return flutterSdkPath + }() + + includeBuild("$flutterSdkPath/packages/flutter_tools/gradle") + + repositories { + google() + mavenCentral() + gradlePluginPortal() + } +} + +plugins { + id "dev.flutter.flutter-plugin-loader" version "1.0.0" + id "com.android.application" version "8.1.0" apply false + id "org.jetbrains.kotlin.android" version "1.8.22" apply false +} + +include ":app" +'''); + + // Run flutter build apk with release mode + final ProcessResult buildApkResult = await processManager.run([ + flutterBin, + ...getLocalEngineArguments(), + 'build', + 'apk', + '--release', + ], workingDirectory: appDir.path); + + // Build should succeed and not throw any errors about settings.gradle + expect(buildApkResult.stderr.toString(), + isNot(contains('Please fix your settings.gradle'))); + expect(buildApkResult, const ProcessResultMatcher()); + }); +}