Version 2.11.0-273.0.dev Merge commit '2fafb4061fefdd466bcdbef00e071a079e7f92d0' into 'dev'
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart index 30ed3d0..0ef80e5 100644 --- a/pkg/analyzer/lib/error/error.dart +++ b/pkg/analyzer/lib/error/error.dart
@@ -16,6 +16,7 @@ import 'package:analyzer/src/generated/parser.dart' show ParserErrorCode; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/manifest/manifest_warning_code.dart'; +import 'package:analyzer/src/pubspec/pubspec_warning_code.dart'; export 'package:_fe_analyzer_shared/src/base/errors.dart' show ErrorCode, ErrorSeverity, ErrorType; @@ -795,6 +796,9 @@ ParserErrorCode.WITH_BEFORE_EXTENDS, ParserErrorCode.WRONG_SEPARATOR_FOR_POSITIONAL_PARAMETER, ParserErrorCode.WRONG_TERMINATOR_FOR_PARAMETER_GROUP, + PubspecWarningCode.INVALID_DEPENDENCY, + PubspecWarningCode.PATH_DOES_NOT_EXIST, + PubspecWarningCode.PATH_PUBSPEC_DOES_NOT_EXIST, ScannerErrorCode.EXPECTED_TOKEN, ScannerErrorCode.ILLEGAL_CHARACTER, ScannerErrorCode.MISSING_DIGIT,
diff --git a/pkg/analyzer/lib/src/dart/micro/libraries_log.dart b/pkg/analyzer/lib/src/dart/micro/libraries_log.dart deleted file mode 100644 index a67988b..0000000 --- a/pkg/analyzer/lib/src/dart/micro/libraries_log.dart +++ /dev/null
@@ -1,88 +0,0 @@ -// Copyright (c) 2020, 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. - -import 'package:meta/meta.dart'; - -class ChangeFileLoadEntry extends LibrariesLogEntry { - /// The path of the file that was reported as changed. - final String target; - - /// The files that depend transitively on the [target]. These files are - /// removed from the state, and from the element factory. - final List<LibrariesLogFile> removed = []; - - ChangeFileLoadEntry._(this.target); - - void addRemoved({ - @required String path, - @required Uri uri, - }) { - removed.add( - LibrariesLogFile._(path, uri), - ); - } - - @override - String toString() { - return 'Change(target: $target, removed: $removed)'; - } -} - -class LibrariesLog { - final List<LibrariesLogEntry> entries = []; - - ChangeFileLoadEntry changeFile(String path) { - var entry = ChangeFileLoadEntry._(path); - entries.add(entry); - return entry; - } - - LoadLibrariesForTargetLogEntry loadForTarget({ - @required String path, - @required Uri uri, - }) { - var entry = LoadLibrariesForTargetLogEntry._( - LibrariesLogFile._(path, uri), - ); - entries.add(entry); - return entry; - } -} - -abstract class LibrariesLogEntry { - final DateTime time = DateTime.now(); -} - -class LibrariesLogFile { - final String path; - final Uri uri; - - LibrariesLogFile._(this.path, this.uri); - - @override - String toString() { - return '(path: $path, uri: $uri)'; - } -} - -class LoadLibrariesForTargetLogEntry extends LibrariesLogEntry { - final LibrariesLogFile target; - final List<LibrariesLogFile> loaded = []; - - LoadLibrariesForTargetLogEntry._(this.target); - - void addLibrary({ - @required String path, - @required Uri uri, - }) { - loaded.add( - LibrariesLogFile._(path, uri), - ); - } - - @override - String toString() { - return 'Load(target: $target, loaded: $loaded)'; - } -}
diff --git a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart index b77c5aa..b0e1808 100644 --- a/pkg/analyzer/lib/src/dart/micro/resolve_file.dart +++ b/pkg/analyzer/lib/src/dart/micro/resolve_file.dart
@@ -20,7 +20,6 @@ import 'package:analyzer/src/dart/analysis/results.dart'; import 'package:analyzer/src/dart/micro/analysis_context.dart'; import 'package:analyzer/src/dart/micro/cider_byte_store.dart'; -import 'package:analyzer/src/dart/micro/libraries_log.dart'; import 'package:analyzer/src/dart/micro/library_analyzer.dart'; import 'package:analyzer/src/dart/micro/library_graph.dart'; import 'package:analyzer/src/exception/exception.dart'; @@ -76,13 +75,11 @@ MicroContextObjects contextObjects; - final LibrariesLog _librariesLog = LibrariesLog(); - _LibraryContext libraryContext; /// List of ids for cache elements that are invalidated. Track elements that - /// are invalidated during [changeFile]. Used in [releaseAndClearRemovedIds] to - /// release the cache items and is then cleared. + /// are invalidated during [changeFile]. Used in [releaseAndClearRemovedIds] + /// to release the cache items and is then cleared. final Set<int> removedCacheIds = {}; FileResolver( @@ -124,10 +121,6 @@ this.byteStore = byteStore; } - List<LibrariesLogEntry> get librariesLogEntries { - return _librariesLog.entries; - } - /// Update the resolver to reflect the fact that the file with the given /// [path] was changed. We need to make sure that when this file, of any file /// that directly or indirectly referenced it, is resolved, we used the new @@ -142,13 +135,8 @@ var removedFiles = <FileState>[]; fsState.changeFile(path, removedFiles); - // Update the log. - var logEntry = _librariesLog.changeFile(path); + // Schedule disposing references to cached unlinked data. for (var removedFile in removedFiles) { - logEntry.addRemoved( - path: removedFile.path, - uri: removedFile.uri, - ); removedCacheIds.add(removedFile.id); } @@ -474,7 +462,6 @@ resourceProvider, byteStore, contextObjects, - _librariesLog, ); } } @@ -615,7 +602,6 @@ final ResourceProvider resourceProvider; final CiderByteStore byteStore; final MicroContextObjects contextObjects; - final LibrariesLog librariesLog; LinkedElementFactory elementFactory; @@ -626,7 +612,6 @@ this.resourceProvider, this.byteStore, this.contextObjects, - this.librariesLog, ) { elementFactory = LinkedElementFactory( contextObjects.analysisContext, @@ -646,21 +631,9 @@ var librariesLinkedTimer = Stopwatch(); var inputsTimer = Stopwatch(); - var logEntry = librariesLog.loadForTarget( - path: targetLibrary.path, - uri: targetLibrary.uri, - ); - void loadBundle(LibraryCycle cycle) { if (!loadedBundles.add(cycle)) return; - for (var library in cycle.libraries) { - logEntry.addLibrary( - path: library.path, - uri: library.uri, - ); - } - performance.getDataInt('cycleCount').increment(); performance.getDataInt('libraryCount').add(cycle.libraries.length);
diff --git a/pkg/analyzer/lib/src/pubspec/pubspec_validator.dart b/pkg/analyzer/lib/src/pubspec/pubspec_validator.dart index 83c2031..af82bcf 100644 --- a/pkg/analyzer/lib/src/pubspec/pubspec_validator.dart +++ b/pkg/analyzer/lib/src/pubspec/pubspec_validator.dart
@@ -8,6 +8,7 @@ import 'package:analyzer/src/generated/engine.dart'; import 'package:analyzer/src/generated/source.dart'; import 'package:analyzer/src/pubspec/pubspec_warning_code.dart'; +import 'package:analyzer/src/util/yaml.dart'; import 'package:path/path.dart' as path; import 'package:source_span/src/span.dart'; import 'package:yaml/yaml.dart'; @@ -27,12 +28,21 @@ /// configuration data. static const String FLUTTER_FIELD = 'flutter'; + /// The name of the field whose value is a git dependency. + static const String GIT_FIELD = 'git'; + /// The name of the field whose value is the name of the package. static const String NAME_FIELD = 'name'; /// The name of the field whose value is a path to a package dependency. static const String PATH_FIELD = 'path'; + /// The name of the field whose value is the where to publish the package. + static const String PUBLISH_TO_FIELD = 'publish_to'; + + /// The name of the field whose value is the version of the package. + static const String VERSION_FIELD = 'version'; + /// The resource provider used to access the file system. final ResourceProvider provider; @@ -89,6 +99,16 @@ return false; } + String _asString(dynamic node) { + if (node is String) { + return node; + } + if (node is YamlScalar && node.value is String) { + return node.value as String; + } + return null; + } + /// Return a map whose keys are the names of declared dependencies and whose /// values are the specifications of those dependencies. The map is extracted /// from the given [contents] using the given [key]. @@ -122,8 +142,17 @@ Map<dynamic, YamlNode> declaredDevDependencies = _getDeclaredDependencies(reporter, contents, DEV_DEPENDENCIES_FIELD); + bool isPublishablePackage = false; + var version = contents[VERSION_FIELD]; + if (version != null) { + var publishTo = _asString(contents[PUBLISH_TO_FIELD]); + if (publishTo != 'none') { + isPublishablePackage = true; + } + } + for (var dependency in declaredDependencies.entries) { - _validatePathEntries(reporter, dependency.value); + _validatePathEntries(reporter, dependency.value, isPublishablePackage); } for (var dependency in declaredDevDependencies.entries) { @@ -132,7 +161,7 @@ _reportErrorForNode(reporter, packageName, PubspecWarningCode.UNNECESSARY_DEV_DEPENDENCY, [packageName.value]); } - _validatePathEntries(reporter, dependency.value); + _validatePathEntries(reporter, dependency.value, false); } } @@ -209,33 +238,44 @@ /// Valid paths are directories that: /// /// 1. exist, - /// 2. contain a pubspec.yaml file, and - /// 3. `lib` dir - void _validatePathEntries(ErrorReporter reporter, YamlNode dependency) { + /// 2. contain a pubspec.yaml file + /// + /// If [checkForPathAndGitDeps] is true, `git` or `path` dependencies will + /// be marked invalid. + void _validatePathEntries(ErrorReporter reporter, YamlNode dependency, + bool checkForPathAndGitDeps) { if (dependency is YamlMap) { - for (var node in dependency.nodes.entries) { - var pathEntry = dependency[PATH_FIELD]; - if (pathEntry != null) { - var context = provider.pathContext; - var normalizedPath = context.joinAll(path.posix.split(pathEntry)); - var packageRoot = context.dirname(source.fullName); - var dependencyPath = context.join(packageRoot, normalizedPath); - dependencyPath = context.absolute(dependencyPath); - dependencyPath = context.normalize(dependencyPath); - var packageFolder = provider.getFolder(dependencyPath); - if (!packageFolder.exists) { - _reportErrorForNode(reporter, node.value, - PubspecWarningCode.PATH_DOES_NOT_EXIST, [pathEntry]); - } else { - if (!packageFolder - .getChild(AnalysisEngine.PUBSPEC_YAML_FILE) - .exists) { - _reportErrorForNode(reporter, node.value, - PubspecWarningCode.PATH_PUBSPEC_DOES_NOT_EXIST, [pathEntry]); - } - // todo (pq): test for presence of a lib dir. + var pathEntry = _asString(dependency[PATH_FIELD]); + if (pathEntry != null) { + YamlNode pathKey() => getKey(dependency, PATH_FIELD); + var context = provider.pathContext; + var normalizedPath = context.joinAll(path.posix.split(pathEntry)); + var packageRoot = context.dirname(source.fullName); + var dependencyPath = context.join(packageRoot, normalizedPath); + dependencyPath = context.absolute(dependencyPath); + dependencyPath = context.normalize(dependencyPath); + var packageFolder = provider.getFolder(dependencyPath); + if (!packageFolder.exists) { + _reportErrorForNode(reporter, pathKey(), + PubspecWarningCode.PATH_DOES_NOT_EXIST, [pathEntry]); + } else { + if (!packageFolder + .getChild(AnalysisEngine.PUBSPEC_YAML_FILE) + .exists) { + _reportErrorForNode(reporter, pathKey(), + PubspecWarningCode.PATH_PUBSPEC_DOES_NOT_EXIST, [pathEntry]); } } + if (checkForPathAndGitDeps) { + _reportErrorForNode(reporter, pathKey(), + PubspecWarningCode.INVALID_DEPENDENCY, [PATH_FIELD]); + } + } + + var gitEntry = dependency[GIT_FIELD]; + if (gitEntry != null && checkForPathAndGitDeps) { + _reportErrorForNode(reporter, getKey(dependency, GIT_FIELD), + PubspecWarningCode.INVALID_DEPENDENCY, [GIT_FIELD]); } } }
diff --git a/pkg/analyzer/lib/src/pubspec/pubspec_warning_code.dart b/pkg/analyzer/lib/src/pubspec/pubspec_warning_code.dart index ecc7124..a93cca0 100644 --- a/pkg/analyzer/lib/src/pubspec/pubspec_warning_code.dart +++ b/pkg/analyzer/lib/src/pubspec/pubspec_warning_code.dart
@@ -52,6 +52,17 @@ "The value of the 'flutter' field is expected to be a map.", correction: "Try converting the value to be a map."); + /// A code indicating that a versioned package has an invalid dependency (git + /// or path). + /// + /// Parameters: + /// 0: the kind of dependency. + static const PubspecWarningCode INVALID_DEPENDENCY = PubspecWarningCode( + 'INVALID_DEPENDENCY', "Publishable packages can't have {0} dependencies.", + correction: + "Try adding a 'publish_to: none' entry to mark the package as not " + "for publishing or remove the {0} dependency."); + /// A code indicating that the name field is missing. static const PubspecWarningCode MISSING_NAME = PubspecWarningCode( 'MISSING_NAME', "The name field is required but missing.",
diff --git a/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart b/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart index 327276b..549701d 100644 --- a/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart +++ b/pkg/analyzer/test/src/dart/micro/simple_file_resolver_test.dart
@@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/src/dart/error/syntactic_errors.dart'; -import 'package:analyzer/src/dart/micro/libraries_log.dart'; import 'package:analyzer/src/error/codes.dart'; import 'package:analyzer/src/lint/registry.dart'; import 'package:matcher/matcher.dart'; @@ -25,10 +24,6 @@ String bPath; String cPath; - String get _asyncLibraryPath => futureElement.library.source.fullName; - - String get _coreLibraryPath => intElement.library.source.fullName; - @override void setUp() { super.setUp(); @@ -37,57 +32,6 @@ cPath = convertPath('/workspace/dart/test/lib/c.dart'); } - test_changeFile_log() async { - newFile(aPath, content: r''' -class A {} -'''); - - newFile(bPath, content: r''' -import 'a.dart'; -A a; -B b; -'''); - - result = await resolveFile(bPath); - assertErrorsInResolvedUnit(result, [ - error(CompileTimeErrorCode.UNDEFINED_CLASS, 22, 1), - ]); - - newFile(aPath, content: r''' -class A {} -class B {} -'''); - fileResolver.changeFile(aPath); - - result = await resolveFile(bPath); - assertErrorsInResolvedUnit(result, []); - - // The failure of this check will be reported badly. - expect(fileResolver.librariesLogEntries, [ - predicate((LoadLibrariesForTargetLogEntry entry) { - expect(entry.target.path, bPath); - var loadedPathSet = entry.loaded.map((f) => f.path).toSet(); - expect(loadedPathSet, contains(aPath)); - expect(loadedPathSet, contains(bPath)); - expect(loadedPathSet, contains(_asyncLibraryPath)); - expect(loadedPathSet, contains(_coreLibraryPath)); - return true; - }), - predicate((ChangeFileLoadEntry entry) { - expect(entry.target, aPath); - var removedPathSet = entry.removed.map((f) => f.path).toSet(); - expect(removedPathSet, {aPath, bPath}); - return true; - }), - predicate((LoadLibrariesForTargetLogEntry entry) { - expect(entry.target.path, bPath); - var loadedPathSet = entry.loaded.map((f) => f.path).toSet(); - expect(loadedPathSet, {aPath, bPath}); - return true; - }), - ]); - } - test_changeFile_refreshedFiles() async { newFile(aPath, content: r''' class A {}
diff --git a/pkg/analyzer/test/src/pubspec/pubspec_validator_test.dart b/pkg/analyzer/test/src/pubspec/pubspec_validator_test.dart index 32f7cf3..8e8944a 100644 --- a/pkg/analyzer/test/src/pubspec/pubspec_validator_test.dart +++ b/pkg/analyzer/test/src/pubspec/pubspec_validator_test.dart
@@ -196,6 +196,73 @@ '''); } + test_dependencyGit_malformed_empty() { + // todo (pq): consider validating. + assertNoErrors(''' +name: sample +dependencies: + foo: + git: +'''); + } + + test_dependencyGit_malformed_list() { + // todo (pq): consider validating. + assertNoErrors(''' +name: sample +dependencies: + foo: + git: + - baz +'''); + } + + test_dependencyGit_malformed_scalar() { + // todo (pq): consider validating. + assertNoErrors(''' +name: sample +dependencies: + foo: + git: baz +'''); + } + + test_dependencyGit_noVersion_valid() { + assertNoErrors(''' +name: sample +dependencies: + foo: + git: + url: git@github.com:foo/foo.git + path: path/to/foo +'''); + } + + test_dependencyGit_version_error() { + assertErrors(''' +name: sample +version: 0.1.0 +dependencies: + foo: + git: + url: git@github.com:foo/foo.git + path: path/to/foo +''', [PubspecWarningCode.INVALID_DEPENDENCY]); + } + + test_dependencyGit_version_valid() { + assertNoErrors(''' +name: sample +version: 0.1.0 +publish_to: none +dependencies: + foo: + git: + url: git@github.com:foo/foo.git + path: path/to/foo +'''); + } + test_dependencyGitPath() { // git paths are not validated assertNoErrors(''' @@ -208,6 +275,40 @@ '''); } + test_dependencyPath_malformed_empty() { + // todo (pq): consider validating. + assertNoErrors(''' +name: sample +dependencies: + foo: + path: +'''); + } + + test_dependencyPath_malformed_list() { + // todo (pq): consider validating. + assertNoErrors(''' +name: sample +dependencies: + foo: + path: + - baz +'''); + } + + test_dependencyPath_noVersion_valid() { + newFolder('/foo'); + newFile('/foo/pubspec.yaml', content: ''' +name: foo +'''); + assertNoErrors(''' +name: sample +dependencies: + foo: + path: /foo +'''); + } + test_dependencyPath_pubspecDoesNotExist() { newFolder('/foo'); assertErrors(''' @@ -257,6 +358,35 @@ '''); } + test_dependencyPath_version_error() { + newFolder('/foo'); + newFile('/foo/pubspec.yaml', content: ''' +name: foo +'''); + assertErrors(''' +name: sample +version: 0.1.0 +dependencies: + foo: + path: /foo +''', [PubspecWarningCode.INVALID_DEPENDENCY]); + } + + test_dependencyPath_version_valid() { + newFolder('/foo'); + newFile('/foo/pubspec.yaml', content: ''' +name: foo +'''); + assertNoErrors(''' +name: sample +version: 0.1.0 +publish_to: none +dependencies: + foo: + path: /foo +'''); + } + test_dependencyPathDoesNotExist_path_error() { assertErrors(''' name: sample @@ -288,6 +418,19 @@ '''); } + test_devDependencyGit_version_no_error() { + // Git paths are OK in dev_dependencies + assertNoErrors(''' +name: sample +version: 0.1.0 +dev_dependencies: + foo: + git: + url: git@github.com:foo/foo.git + path: path/to/foo +'''); + } + test_devDependencyPathDoesNotExist_path_error() { assertErrors(''' name: sample
diff --git a/tools/VERSION b/tools/VERSION index ba1203e..6a2eb32 100644 --- a/tools/VERSION +++ b/tools/VERSION
@@ -27,5 +27,5 @@ MAJOR 2 MINOR 11 PATCH 0 -PRERELEASE 272 +PRERELEASE 273 PRERELEASE_PATCH 0 \ No newline at end of file