[tool] Only license-check checked-in files (#9905)

Instead of checking every file on the filesystem in the repo directory when checking licenses, only check the checked-in files. This avoids the problem of having many, many false positives when running locally if example apps have been built, since many builds create git-ignored files (generated files, downloaded Pods, etc.) that aren't our source, and so don't need to (and don't) pass the check that they contain our license block.

Author: stuartmorgan-g

script/tool/lib/src/license_check_command.dart

@@ -2,13 +2,18 @@
 // 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:file/file.dart';
+import 'package:git/git.dart';
 import 'package:path/path.dart' as p;
 
 import 'common/core.dart';
 import 'common/output_utils.dart';
 import 'common/package_command.dart';
 
+const int _exitListFilesFailed = 3;
+
 const Set<String> _codeFileExtensions = <String>{
   '.c',
   '.cc',
@@ -44,11 +49,6 @@ const Set<String> _ignoredFullBasenameList = <String>{
   'resource.h', // Generated by VS.
 };
 
-// Path parts to ignore. Used to ignore entire subdirectories.
-const Set<String> _ignorePathPartList = <String>{
-  'FlutterGeneratedPluginSwiftPackage', // Generated by Flutter tool.
-};
-
 // Third-party packages where the code doesn't have file-level annotation, just
 // the package-level LICENSE file. Each entry must be a directory relative to
 // third_party/packages, as that is the only directory where this is allowed.
@@ -152,7 +152,7 @@ class LicenseCheckCommand extends PackageCommand {
         .map(
             (Directory dir) => '${dir.absolute.path}${platform.pathSeparator}');
 
-    final Iterable<File> allFiles = (await _getAllFiles()).where(
+    final Iterable<File> allFiles = (await _getAllCheckedInFiles()).where(
         (File file) => !submodulePaths.any(file.absolute.path.startsWith));
 
     final Iterable<File> codeFiles = allFiles.where((File file) =>
@@ -175,7 +175,7 @@ class LicenseCheckCommand extends PackageCommand {
       printError(
           'The following LICENSE files do not follow the expected format:');
       for (final File file in licenseFileFailures) {
-        printError('  ${file.path}');
+        printError('  ${_repoRelativePath(file)}');
       }
       printError('Please ensure that they use the exact format used in this '
           'repository".\n');
@@ -186,7 +186,7 @@ class LicenseCheckCommand extends PackageCommand {
       printError('The license block for these files is missing or incorrect:');
       for (final File file
           in codeFileFailures[_LicenseFailureType.incorrectFirstParty]!) {
-        printError('  ${file.path}');
+        printError('  ${_repoRelativePath(file)}');
       }
       printError(
           'If this third-party code, move it to a "third_party/" directory, '
@@ -200,7 +200,7 @@ class LicenseCheckCommand extends PackageCommand {
           'No recognized license was found for the following third-party files:');
       for (final File file
           in codeFileFailures[_LicenseFailureType.unknownThirdParty]!) {
-        printError('  ${file.path}');
+        printError('  ${_repoRelativePath(file)}');
       }
       print('Please check that they have a license at the top of the file. '
           'If they do, the license check needs to be updated to recognize '
@@ -242,7 +242,7 @@ class LicenseCheckCommand extends PackageCommand {
     };
 
     for (final File file in codeFiles) {
-      print('Checking ${file.path}');
+      print('Checking ${_repoRelativePath(file)}');
       // Some third-party directories have code that doesn't annotate each file,
       // so for those check the LICENSE file instead. This is done even though
       // it's redundant to re-check it for each file because it ensures that we
@@ -298,7 +298,7 @@ class LicenseCheckCommand extends PackageCommand {
     final List<File> incorrectLicenseFiles = <File>[];
 
     for (final File file in files) {
-      print('Checking ${file.path}');
+      print('Checking ${_repoRelativePath(file)}');
       // On Windows, git may auto-convert line endings on checkout; this should
       // still pass since they will be converted back on commit.
       final String contents = file.readAsStringSync().replaceAll('\r\n', '\n');
@@ -323,23 +323,30 @@ class LicenseCheckCommand extends PackageCommand {
       return true;
     }
 
-    final List<String> parts = path.split(file.path);
-    if (parts.any(_ignorePathPartList.contains)) {
-      return true;
-    }
-
     return false;
   }
 
   bool _isThirdParty(File file) {
     return path.split(file.path).contains('third_party');
   }
 
-  Future<List<File>> _getAllFiles() => packagesDir.parent
-      .list(recursive: true, followLinks: false)
-      .where((FileSystemEntity entity) => entity is File)
-      .map((FileSystemEntity file) => file as File)
-      .toList();
+  Future<Iterable<File>> _getAllCheckedInFiles() async {
+    final GitDir git = await gitDir;
+    final ProcessResult result =
+        await git.runCommand(<String>['ls-files'], throwOnError: false);
+    if (result.exitCode != 0) {
+      printError('Unable to get list of files under source control');
+      throw ToolExit(_exitListFilesFailed);
+    }
+    final Directory repoRoot = packagesDir.parent;
+    return (result.stdout as String)
+        .trim()
+        .split('\n')
+        .where((String path) => path.isNotEmpty)
+        .map((String path) => repoRoot.childFile(path))
+        // Filter out symbolic links to avoid checking files multiple times.
+        .where((File f) => !repoRoot.fileSystem.isLinkSync(f.path));
+  }
 
   // Returns the directories containing mapped submodules, if any.
   Future<Iterable<Directory>> _getSubmoduleDirectories() async {
@@ -358,6 +365,11 @@ class LicenseCheckCommand extends PackageCommand {
     }
     return submodulePaths;
   }
+
+  String _repoRelativePath(File file) {
+    return p.posix.joinAll(path.split(
+        path.relative(file.absolute.path, from: packagesDir.parent.path)));
+  }
 }
 
 enum _LicenseFailureType { incorrectFirstParty, unknownThirdParty }

script/tool/test/license_check_command_test.dart

@@ -7,6 +7,7 @@ import 'package:file/file.dart';
 import 'package:flutter_plugin_tools/src/common/core.dart';
 import 'package:flutter_plugin_tools/src/license_check_command.dart';
 import 'package:git/git.dart';
+import 'package:path/path.dart' as p;
 import 'package:platform/platform.dart';
 import 'package:test/test.dart';
 
@@ -17,13 +18,14 @@ void main() {
   group('LicenseCheckCommand', () {
     late CommandRunner<void> runner;
     late Platform platform;
+    late RecordingProcessRunner gitProcessRunner;
     late Directory packagesDir;
     late Directory root;
 
     setUp(() {
       platform = MockPlatformWithSeparator();
       final GitDir gitDir;
-      (:packagesDir, processRunner: _, gitProcessRunner: _, :gitDir) =
+      (:packagesDir, processRunner: _, :gitProcessRunner, :gitDir) =
           configureBaseCommandMocks(platform: platform);
       root = packagesDir.parent;
 
@@ -64,6 +66,22 @@ void main() {
       file.writeAsStringSync(lines.join(newline) + suffix + newline);
     }
 
+    /// Mocks `git ls-files` to return all files in `root`, simulating a
+    /// repository where everything present is checked in.
+    void mockGitFilesListWithAllFiles(Directory root) {
+      final String fileList = root
+          .listSync(recursive: true, followLinks: false)
+          .whereType<File>()
+          .map((File f) => p.posix
+              .joinAll(p.split(p.relative(f.absolute.path, from: root.path))))
+          .join('\n');
+
+      gitProcessRunner.mockProcessesForExecutable['git-ls-files'] =
+          <FakeProcessInfo>[
+        FakeProcessInfo(MockProcess(stdout: '$fileList\n')),
+      ];
+    }
+
     test('looks at only expected extensions', () async {
       final Map<String, bool> extensions = <String, bool>{
         'c': true,
@@ -88,6 +106,7 @@ void main() {
       for (final String fileExtension in extensions.keys) {
         root.childFile('$filenameBase.$fileExtension').createSync();
       }
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output = await runCapturingPrint(
           runner, <String>['license-check'], errorHandler: (Error e) {
@@ -121,6 +140,7 @@ void main() {
       for (final String name in ignoredFiles) {
         root.childFile(name).createSync();
       }
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -157,7 +177,9 @@ void main() {
       }
     });
 
-    test('ignores FlutterGeneratedPluginSwiftPackage', () async {
+    test('ignores files that are not checked in', () async {
+      mockGitFilesListWithAllFiles(root);
+      // Add files after creating the mock output from created files.
       final Directory packageDir = root
           .childDirectory('FlutterGeneratedPluginSwiftPackage')
         ..createSync();
@@ -181,6 +203,7 @@ void main() {
       writeLicense(checked);
       final File notChecked = root.childFile('not_checked.md');
       notChecked.createSync();
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -198,6 +221,7 @@ void main() {
       final File checked = root.childFile('checked.cc');
       checked.createSync();
       writeLicense(checked, useCrlf: true);
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -221,6 +245,7 @@ void main() {
       final File fileC = root.childFile('file_c.html');
       fileC.createSync();
       writeLicense(fileC, comment: '', prefix: '<!-- ', suffix: ' -->');
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -245,6 +270,7 @@ void main() {
       writeLicense(goodB);
       root.childFile('bad.cc').createSync();
       root.childFile('bad.h').createSync();
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -273,6 +299,7 @@ void main() {
       final File bad = root.childFile('bad.cc');
       bad.createSync();
       writeLicense(bad, copyright: '');
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -300,6 +327,7 @@ void main() {
       final File bad = root.childFile('bad.cc');
       bad.createSync();
       writeLicense(bad, license: <String>[]);
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -325,6 +353,7 @@ void main() {
       final File thirdPartyFile = root.childFile('third_party.cc');
       thirdPartyFile.createSync();
       writeLicense(thirdPartyFile, copyright: 'Copyright 2017 Someone Else');
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -359,6 +388,7 @@ void main() {
             'Licensed under the Apache License, Version 2.0 (the "License");',
             'you may not use this file except in compliance with the License.'
           ]);
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -381,6 +411,7 @@ void main() {
           .childFile('first_party.cc');
       firstPartyFileInThirdParty.createSync(recursive: true);
       writeLicense(firstPartyFileInThirdParty);
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -404,6 +435,7 @@ void main() {
         'This program is free software: you can redistribute it and/or modify',
         'it under the terms of the GNU General Public License',
       ]);
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -439,6 +471,7 @@ void main() {
           'you may not use this file except in compliance with the License.'
         ],
       );
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -464,6 +497,7 @@ void main() {
       final File license = root.childFile('LICENSE');
       license.createSync();
       license.writeAsStringSync(_correctLicenseFileText);
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -482,6 +516,7 @@ void main() {
       license.createSync();
       license
           .writeAsStringSync(_correctLicenseFileText.replaceAll('\n', '\r\n'));
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -500,6 +535,7 @@ void main() {
       final File license = root.childFile('LICENSE');
       license.createSync();
       license.writeAsStringSync(_incorrectLicenseFileText);
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -516,6 +552,7 @@ void main() {
           root.childDirectory('third_party').childFile('LICENSE');
       license.createSync(recursive: true);
       license.writeAsStringSync(_incorrectLicenseFileText);
+      mockGitFilesListWithAllFiles(root);
 
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);
@@ -533,6 +570,7 @@ void main() {
       final File license = root.childFile('LICENSE');
       license.createSync();
       license.writeAsStringSync(_incorrectLicenseFileText);
+      mockGitFilesListWithAllFiles(root);
 
       Error? commandError;
       final List<String> output = await runCapturingPrint(
@@ -563,7 +601,7 @@ void main() {
       final File checked = root.childFile('Package.swift');
       checked.createSync();
       writeLicense(checked, prefix: '// swift-tools-version: 5.9\n');
-
+      mockGitFilesListWithAllFiles(root);
       final List<String> output =
           await runCapturingPrint(runner, <String>['license-check']);