directives_ordering inconsistent with other orderings

[davidmorgan]

The sort order does not seem to be consistent with analyzer/tidy_dart when there are packages with . in the name; it looks like we need to split the package name out, sort by then, then sort by name within package.

Other suggestions to check:

  import "/src/foo.dart";
  import "./foo.dart";
  import "../foo.dart";  

[pq]

@davidmorgan: are we good on this one now?

[davidmorgan]

IIRC, yes :)

[pq]

Super! I'll ride your enthusiasm and close this one; we can re-open if we hit any snags. Thanks!

/fyi @jefflim-google

[oprypin]

I found another edge case.

What directives_ordering enforces:

import './foo1.dart';
import '../../foo2.dart';
import '../foo3.dart';
import 'foo4.dart';

What I expect & what tidy_dart does:

import '../../foo1.dart';
import '../foo2.dart';
import './foo3.dart';
import 'foo4.dart';

Or alternatively the exact opposite of the latter also might make sense, but not the mixed order.

---

I think the culprit is the fact that this function is invoked on the uri whether or not it is actually from a package, and so the part before the first slash is sorted separately:

linter/lib/src/rules/directives_ordering.dart

Lines 118 to 125 in cf4fbcd

	int compareDirectives(String a, String b) {
	var indexA = a.indexOf('/');
	var indexB = b.indexOf('/');
	if (indexA == -1 || indexB == -1) return a.compareTo(b);
	var result = a.substring(0, indexA).compareTo(b.substring(0, indexB));
	if (result != 0) return result;
	return a.substring(indexA + 1).compareTo(b.substring(indexB + 1));
	}

I.e. it sorts the items as if they are such tuples:

 ('.', 'foo1.dart')
 ('..', '../foo2.dart')
 ('..', 'foo3.dart')

I can't think of any other scenario where this "tuple-ness" causes wrong orderings to happen, although at first it might seem like a wide class of errors. The ASCII table ordering goes like [., /, then letters]. For letters it's correct anyway but specifically the dots are affected.

---

And also I guess one big reason that this error has gone unnoticed is that you're just not supposed to write './foo.dart'. Maybe that's another thing that could be separately considered for enforcement - drop ./ if possible :)

[bwilkerson]

Has this been fixed now?

[oprypin]

I reopened because it was only mostly fixed.

With that, the status is:

@oprypin oprypin linked a pull request Jul 6, 2023 that will close this issue
Fix directives_ordering for a leading dot in path #4538