Fix SR-13490: Fix LT operator and add tests for ImportPath

Author: Interfere

include/swift/AST/Import.h

@@ -22,9 +22,10 @@
 #include "swift/AST/Identifier.h"
 #include "swift/Basic/Located.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include <algorithm>
 
 namespace swift {
 class ASTContext;
@@ -191,6 +192,20 @@ namespace detail {
   };
 }
 
+/// @name ImportPathBase Comparison Operators
+/// @{
+template <typename Subclass>
+inline bool operator<(const detail::ImportPathBase<Subclass> &LHS,
+                      const detail::ImportPathBase<Subclass> &RHS) {
+  using Element = typename detail::ImportPathBase<Subclass>::Element;
+  auto Comparator = [](const Element &l, const Element &r) {
+    return l.Item.compare(r.Item) < 0;
+  };
+  return std::lexicographical_compare(LHS.begin(), LHS.end(), RHS.begin(),
+                                      RHS.end(), Comparator);
+}
+/// @}
+
 /// An undifferentiated series of dotted identifiers in an \c import statement,
 /// like \c Foo.Bar. Each identifier is packaged with its corresponding source
 /// location.

lib/IDE/ModuleInterfacePrinting.cpp

@@ -374,15 +374,7 @@ static bool printModuleInterfaceDecl(Decl *D,
 
 /// Sorts import declarations for display.
 static bool compareImports(ImportDecl *LHS, ImportDecl *RHS) {
-  // TODO(SR-13490): Probably buggy--thinks "import Foo" == "import Foo.Bar".
-  // ImportPathBase should provide universal comparison functions to avoid this.
-  auto LHSPath = LHS->getImportPath();
-  auto RHSPath = RHS->getImportPath();
-  for (unsigned i: range(std::min(LHSPath.size(), RHSPath.size()))) {
-    if (int Ret = LHSPath[i].Item.str().compare(RHSPath[i].Item.str()))
-      return Ret < 0;
-  }
-  return false;
+  return LHS->getImportPath() < RHS->getImportPath();
 };
 
 /// Sorts Swift declarations for display.

unittests/AST/CMakeLists.txt

@@ -6,6 +6,7 @@ add_swift_unittest(SwiftASTTests
   TestContext.cpp
   TypeMatchTests.cpp
   VersionRangeLattice.cpp
+  ImportTests.cpp
 )
 
 target_link_libraries(SwiftASTTests

unittests/AST/ImportTests.cpp

@@ -0,0 +1,60 @@
+//===--- ImportTests.cpp - Tests for representation of imports ------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestContext.h"
+#include "swift/AST/Import.h"
+#include "gtest/gtest.h"
+
+using namespace swift;
+using namespace swift::unittest;
+
+namespace {
+/// Helper class used to create ImportPath and hold all strings for identifiers
+class ImportPathContext {
+  TestContext Ctx;
+
+public:
+  ImportPathContext() = default;
+
+  /// Hepler routine for building ImportPath
+  /// Build()
+  /// @see ImportPathBuilder
+  inline ImportPath Build(StringRef Name) noexcept {
+    return ImportPath::Builder(Ctx.Ctx, Name, '.').copyTo(Ctx.Ctx);
+  }
+};
+
+} // namespace
+
+TEST(ImportPath, Comparison) {
+  ImportPathContext ctx;
+
+  /// Simple sanity check:
+  EXPECT_FALSE(ctx.Build("A.B.C") < ctx.Build("A.B.C"));
+
+  /// Check order chain:
+  /// A < A.A < A.A.A < A.A.B < A.B < A.B.A < AA < B < B.A
+  EXPECT_LT(ctx.Build("A"), ctx.Build("A.A"));
+  EXPECT_LT(ctx.Build("A.A"), ctx.Build("A.A.A"));
+  EXPECT_LT(ctx.Build("A.A.A"), ctx.Build("A.A.B"));
+  EXPECT_LT(ctx.Build("A.A.B"), ctx.Build("A.B"));
+  EXPECT_LT(ctx.Build("A.B"), ctx.Build("A.B.A"));
+  EXPECT_LT(ctx.Build("A.B.A"), ctx.Build("AA"));
+  EXPECT_LT(ctx.Build("B"), ctx.Build("B.A"));
+
+  /// Further ImportPaths are semantically incorrect, but we must
+  /// check that comparing them does not cause compiler to crash.
+  EXPECT_LT(ctx.Build("."), ctx.Build("A"));
+  EXPECT_LT(ctx.Build("A"), ctx.Build("AA."));
+  EXPECT_LT(ctx.Build("A"), ctx.Build("AA.."));
+  EXPECT_LT(ctx.Build(".A"), ctx.Build("AA"));
+}