Skip to content
This repository has been archived by the owner on Nov 20, 2024. It is now read-only.

remove DartProject references #3399

Merged
merged 1 commit into from
May 13, 2022
Merged

remove DartProject references #3399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 28 additions & 34 deletions lib/src/rules/package_prefixed_library_names.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,31 +51,20 @@ library my_package.src.private;
bool matchesOrIsPrefixedBy(String name, String prefix) =>
name == prefix || name.startsWith('$prefix.');

class PackagePrefixedLibraryNames extends LintRule
implements ProjectVisitor, NodeLintRule {
DartProject? project;

class PackagePrefixedLibraryNames extends LintRule {
PackagePrefixedLibraryNames()
: super(
name: 'package_prefixed_library_names',
description: _desc,
details: _details,
group: Group.style);

@override
ProjectVisitor getProjectVisitor() => this;

@override
void registerNodeProcessors(
NodeLintRegistry registry, LinterContext context) {
var visitor = _Visitor(this);
registry.addLibraryDirective(this, visitor);
}

@override
void visit(DartProject project) {
this.project = project;
}
}

class _Visitor extends SimpleAstVisitor<void> {
Expand All @@ -84,28 +73,33 @@ class _Visitor extends SimpleAstVisitor<void> {
_Visitor(this.rule);

@override
// ignore: prefer_expression_function_bodies
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems wrong to recommend using an expression function body when there's no value to return. Somehow void f() => null; feels wrong.

void visitLibraryDirective(LibraryDirective node) {
// If no project info is set, bail early.
// https://github.com/dart-lang/linter/issues/154
var project = rule.project;
var element = node.element;
if (project == null || element == null) {
return;
}

var source = element.source;
if (source == null) {
return;
}

var prefix = Analyzer.facade.createLibraryNamePrefix(
libraryPath: source.fullName,
projectRoot: project.root.absolute.path,
packageName: project.name);

var name = element.name;
if (name == null || !matchesOrIsPrefixedBy(name, prefix)) {
rule.reportLint(node.name);
}
// Project info is not being set.
//See: https://github.com/dart-lang/linter/issues/3395
return;

// // If no project info is set, bail early.
// // https://github.com/dart-lang/linter/issues/154
// var project = rule.project;
// var element = node.element;
// if (project == null || element == null) {
// return;
// }
//
// var source = element.source;
// if (source == null) {
// return;
// }
//
// var prefix = Analyzer.facade.createLibraryNamePrefix(
// libraryPath: source.fullName,
// projectRoot: project.root.absolute.path,
// packageName: project.name);
//
// var name = element.name;
// if (name == null || !matchesOrIsPrefixedBy(name, prefix)) {
// rule.reportLint(node.name);
// }
}
}
3 changes: 2 additions & 1 deletion test_data/rules/package_prefixed_library_names.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@

// test w/ `dart test -N package_prefixed_library_names`

library linter.not_where_it_should_be; //LINT
// See: https://github.com/dart-lang/linter/issues/3395
library linter.not_where_it_should_be; //FAILING
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would commenting out code that isn't being run cause the test to start failing? How confident are we that the commented out code really isn't being run?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is being run in the linter tests since they do take the code path that initializes project instances. That's however the only path that does and so the tests here are out of sync w/ how server and the analyzer CLIs work. Obviously, not good!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: https://github.com/dart-lang/linter/issues/2969