Validate that a package doesn't depend on itself.

Validate that a package doesn't depend on itself.


BUG=7045
Committed: https://code.google.com/p/dart/source/detail?r=16614

[Bob Nystrom, 2013-01-03 20:52:45]

https://codereview.chromium.org/11737013/diff/1/utils/pub/validator/dependenc...
File utils/pub/validator/dependency.dart (right):

https://codereview.chromium.org/11737013/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:38: 'automatically.');
I find the future tense here a bit confusing. How about:

'Pub enables "package:${entrypoint.root.name}" imports implicitly.'

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
File utils/tests/pub/validator_test.dart (right):

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:538: {'hosted': 'foo'}
Shouldn't this be "test_pkg"?

[nweiz, 2013-01-03 21:08:12]

https://codereview.chromium.org/11737013/diff/1/utils/pub/validator/dependenc...
File utils/pub/validator/dependency.dart (right):

https://codereview.chromium.org/11737013/diff/1/utils/pub/validator/dependenc...
utils/pub/validator/dependency.dart:38: 'automatically.');
On 2013/01/03 20:52:45, Bob Nystrom wrote:
> I find the future tense here a bit confusing. How about:
> 
> 'Pub enables "package:${entrypoint.root.name}" imports implicitly.'

Done.

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
File utils/tests/pub/validator_test.dart (right):

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:538: {'hosted': 'foo'}
On 2013/01/03 20:52:45, Bob Nystrom wrote:
> Shouldn't this be "test_pkg"?

Yes. The fact that this didn't have a version constraint caused a different
error, hence the false positive of this test. I've added a version constraint as
well.

[Bob Nystrom, 2013-01-03 21:56:17]

LGTM!

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
File utils/tests/pub/validator_test.dart (right):

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:538: {'hosted': 'foo'}
On 2013/01/03 21:08:12, nweiz wrote:
> On 2013/01/03 20:52:45, Bob Nystrom wrote:
> > Shouldn't this be "test_pkg"?
> 
> Yes. The fact that this didn't have a version constraint caused a different
> error, hence the false positive of this test. I've added a version constraint
as
> well.

That makes me wonder if the expect should be more precise here so you can ensure
you get the validation warning you actually want.

[nweiz, 2013-01-03 22:18:22]

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
File utils/tests/pub/validator_test.dart (right):

https://codereview.chromium.org/11737013/diff/1/utils/tests/pub/validator_tes...
utils/tests/pub/validator_test.dart:538: {'hosted': 'foo'}
On 2013/01/03 21:56:17, Bob Nystrom wrote:
> On 2013/01/03 21:08:12, nweiz wrote:
> > On 2013/01/03 20:52:45, Bob Nystrom wrote:
> > > Shouldn't this be "test_pkg"?
> > 
> > Yes. The fact that this didn't have a version constraint caused a different
> > error, hence the false positive of this test. I've added a version
constraint
> as
> > well.
> 
> That makes me wonder if the expect should be more precise here so you can
ensure
> you get the validation warning you actually want.

I'm not a big fan of searching for strings within error messages. I think having
validator-level granularity will be good enough most of the time.