Import ordering tidy check confused by multiple use statements from same module

[nox]

use url::{Url, UrlParser, SchemeData};
use url::urlutils::{UrlUtils, UrlUtilsWrapper};

./components/script/dom/urlhelper.rs:7: use statement is not in alphabetical order
./components/script/dom/urlhelper.rs:8: use statement is not in alphabetical order

---

use url::{Url, UrlParser};
use url::SchemeData;

./components/script/dom/urlhelper.rs:9: use statement is not in alphabetical order
./components/script/dom/urlhelper.rs:10: use statement is not in alphabetical order

[jdm]

Can this be fixed by reordering the url::{ entries after the others? That would imply that { comes after a-z in alphabetical order.

[nox]

Well, that's why I opened the issue, I don't want to reorder it like this.

[SimonSapin]

We could have a different sorting function, but I’m not sure that it’s worth it.

[nox]

Whether or not it's worth it, I'm not sure a tidy check that is wrong is a good idea.

[SimonSapin]

Is it wrong?

[nox]

use mod::{foo, bar};
use mod::qux;

I don't see why qux should come before {foo, bar}, it makes no sense.

[tafia]

I don't see why qux should come before {foo, bar}, it makes no sense.

It doesn't look that wrong to me.
I'd say simpler first for readability. And a {...} does look less simple than one element.
But I don't have any strong opinion on that matter.

[nox]

use dom::bindings::codegen::InheritTypes::ProcessingInstructionCast;
use dom::bindings::codegen::InheritTypes::{CharacterDataCast, DocumentTypeCast};
use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLScriptElementCast};
use dom::bindings::codegen::InheritTypes::{HTMLFormElementDerived, NodeCast};

ProcessingInstructionCast is at an unexpected place.

[nox]

@tafia The lint is about alphabetic order of imports, not about simplicity first or whatever.

[nox]

qux is after foo and bar in alphabetic order, and thus should come after them, whether there are braces or not.

[nox]

Mmmmh… I can't order foo and bar apparently, good thing I found a real-world example.

[bholley]

Yeah, this is super confusing.

[jdm]

One idea from IRC: lowercase the lines before comparing them so use something::a vs use something::A isn't so confusing.

diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py
index fcf51db..057910e 100644
--- a/python/tidy/servo_tidy/tidy.py
+++ b/python/tidy/servo_tidy/tidy.py
@@ -430,7 +430,7 @@ def check_rust(file_name, lines):
                 yield (idx + 1, "use statement spans multiple lines")
             # strip "use" from the begin and ";" from the end
             current_use = line[4:-1]
-            if indent == current_indent and prev_use and current_use < prev_use:
+            if indent == current_indent and prev_use and current_use.lower() < prev_use.lower():
                 yield(idx + 1, decl_message.format("use statement")
                       + decl_expected.format(prev_use)
                       + decl_found.format(current_use))

This generates a bunch of tidy warnings that need to be addressed.

[nox]

@jdm First time I hear someone complain about this specific part. I'm more annoyed by how braces should come after not-braces.