Deprecate import/module name mismatch (ALL CASES)

changelog/deprecate_bad_module_name.dd

@@ -0,0 +1,48 @@
+Deprecate import module name mismatch
+
+Some instances of module name/import name mismatches were deprecated while others are now considered errors.
+
+Case 1: DEPRECATED: import a module name with a qualified name but the module being imported has no module declaration.
+---
+// main.d
+import foo.bar;
+
+// foo/bar.d
+// this file is empty, it has no module declaration
+---
+The above code will now print:
+
+$(CONSOLE Deprecated: module bar from file foo/bar.d should be imported with 'import bar;' )
+
+Case 2: DEPRECATED: import a module with a qualified name that partially matches the name of the module being imported.
+---
+// main.d
+import foo.bar;
+import foo.baz.buz;
+
+// foo/bar.d
+module bar;
+
+// foo/baz/buz.d
+module baz.buz;
+---
+The above code will now print:
+
+$(CONSOLE Deprecated: module bar from file foo/bar.d should be imported with 'import bar;'
+Deprecated: module baz.buz from file foo/baz/buz.d should be imported with 'import baz.buz;' )
+
+Note that for this rule to apply, the shorter name must completely match the end of the longer name.
+
+Case 3: ERROR: import a module that matches the filename but does not match the module name.
+---
+// main.d
+import foo;
+
+// foo.d
+module bar;
+---
+The above code will now fail and print:
+
+$(CONSOLE Error: module bar from file foo.d must be imported with 'import bar;')
+
+Note that importing a module whose module name does not match its filename is still supported.

src/dmd/dmodule.d

@@ -31,6 +31,7 @@ import dmd.globals;
 import dmd.id;
 import dmd.identifier;
 import dmd.parse;
+import dmd.root.array;
 import dmd.root.file;
 import dmd.root.filename;
 import dmd.root.outbuffer;
@@ -514,7 +515,49 @@ extern (C++) final class Module : Package
             buf.printf("%s\t(%s)", ident.toChars(), m.srcfile.toChars());
             message("import    %s", buf.peekString());
         }
-        m = m.parse();
+        {
+            auto originalModule = m;
+            m = m.parse();
+
+            // verify the module name matches the imported module name
+            if (m is originalModule)
+            {
+                if (m.md is null)
+                {
+                    // in this case there was no module declaration, this means the import
+                    // should have no package prefix, but for now we support this with a
+                    // depecration message for backwards compatibility
+                    if (packages && packages.dim > 0)
+                        m.deprecation(loc, "from file %s should be imported with 'import %s;'",
+                            m.srcfile.name.toChars(), m.toPrettyChars());
+                }
+                else if (!packages.opEquals(m.md.packages) || ident != m.md.id)
+                {
+                    // deprecate the case when the ends of the names match
+                    bool deprecate = false;
+                    if (ident == m.md.id)
+                    {
+                        deprecate = true;
+                        auto mdPackageCount = (m.md.packages == null) ? 0 : m.md.packages.dim;
+                        auto packageCount   = (packages      == null) ? 0 : packages.dim;
+                        for (size_t i = 1; i <= mdPackageCount && i <= packageCount; i++)
+                        {
+                            if (m.md.packages[mdPackageCount - i] != packages[packageCount - i])
+                            {
+                                deprecate = false;
+                                break;
+                            }
+                        }
+                    }
+                    if (deprecate)
+                        m.deprecation(loc, "from file %s should be imported with 'import %s;'",
+                            m.srcfile.name.toChars(), m.toPrettyChars());
+                    else
+                        m.error(loc, "from file %s must be imported with 'import %s;'",
+                            m.srcfile.name.toChars(), m.toPrettyChars());
+                }
+            }
+        }
 
         // Call onImport here because if the module is going to be compiled then we
         // need to determine it early because it affects semantic analysis. This is

src/dmd/root/array.d

@@ -313,3 +313,22 @@ unittest
     array[2] = 910;
     assert([123, 421, 910, 123, 1, 2, 8, 20, 4, 3] == array.asDArray);
 }
+
+/**
+ * Returns true if every element of $(D left) and $(D right) are equal.
+ * Note that an array of size 0 is considered equivalent to a `null` pointer.
+ * Params:
+ *  left = pointer to array to compare
+ *  right = pointer to array to compare
+ */
+bool opEquals(T)(const(Array!T)* left, const(Array!T)* right)
+{
+    if (left is null || left.dim == 0)
+        return right is null || right.dim == 0;
+    if (right is null || left.dim != right.dim)
+        return false;
+    foreach (i; 0 .. left.dim)
+        if ((*left)[i] != (*right)[i])
+            return false;
+    return true;
+}

test/compilable/extra-files/test14894a.d

@@ -1,4 +1,4 @@
-module imports.test14894a;
+module test14894a;
 
 mixin template Protocol()
 {

test/compilable/imports/a313templatemixin1.d

@@ -1,3 +1,4 @@
+module imports.a313templatemixin1;
 void bug()
 {
 }

test/compilable/imports/a313templatemixin2.d

@@ -1,3 +1,4 @@
+module imports.a313templatemixin2;
 void bug()
 {
 }

test/compilable/imports/checkimports3a.d

@@ -1 +1,2 @@
+module imports.checkimports3a;
 void foo() {}

test/compilable/imports/checkimports3b.d

@@ -1 +1,2 @@
+module imports.checkimports3b;
 private void foo(int) {}

test/compilable/imports/checkimports3c.d

@@ -1 +1,2 @@
+module imports.checkimports3c;
 void foo(string) {}

test/compilable/imports/fwdref12201a.d

@@ -1 +1,2 @@
+module imports.fwdref12201a;
 alias int FILE;

test/compilable/imports/fwdref2_test17548.d

@@ -1,9 +1,9 @@
-module fwdref2_test17548;
+module imports.fwdref2_test17548;
 
 import test17548;
 
 struct S2 {
     void bar(int arg = .test17548.cnst) {}
     S1 s;
-    import fwdref2_test17548;
+    import imports.fwdref2_test17548;
 }

test/compilable/imports/fwdref9514.d

@@ -1,3 +1,4 @@
+module imports.fwdref9514;
 bool find9514(alias pred, R)(R range)
 {
     return true;

test/compilable/imports/g313public.d

@@ -1,3 +1,4 @@
+module imports.g313public;
 void bug()
 {
 }

test/compilable/imports/g313staticif.d

@@ -1,3 +1,4 @@
+module imports.g313staticif;
 void bug()
 {
 }

test/compilable/imports/g313stringmixin.d

@@ -1,3 +1,4 @@
+module imports.g313stringmixin;
 void bug()
 {
 }

test/compilable/imports/g313templatemixin.d

@@ -1,3 +1,4 @@
+module imports.g313templatemixin;
 void bug()
 {
 }

test/compilable/imports/imp16085.d

@@ -1,3 +1,4 @@
+module imports.imp16085;
 struct Pass
 {
 }

test/compilable/imports/imp16085b.d

@@ -1,4 +1,5 @@
-import imp16085 : S;
+module imports.imp16085b;
+import imports.imp16085 : S;
 
 struct Fail
 {

test/compilable/imports/protectionimp.d

@@ -1,3 +1,4 @@
+module imports.protectionimp;
 private
 {
     void privF() {}

test/compilable/imports/test11563core_bitop.d

@@ -1 +1 @@
-module test11563core_bitop;
+module imports.test11563core_bitop;

test/compilable/imports/test15857a.d

@@ -1,2 +1,3 @@
+module imports.test15857a;
 public import imports.test15857b;
 public import imports.test15857c;

test/compilable/imports/test15857b.d

@@ -1 +1,2 @@
+module imports.test15857b;
 void bar15857(int) {}

test/compilable/imports/test15857c.d

@@ -1 +1,2 @@
+module imports.test15857c;
 void bar15857(string) {}

test/compilable/imports/test71.d

@@ -1,4 +1,4 @@
-module imports_test71;
+module imports.test71;
 import imports = object;
 
 void foo()

test/compilable/test15925.d

@@ -1,8 +1,9 @@
-/* REQUIRED_ARGS: -transition=import -transition=checkimports
+/* REQUIRED_ARGS: -transition=import -transition=checkimports -Icompilable/imports
 PERMUTE_ARGS:
 TEST_OUTPUT:
 ---
-compilable/test15925.d(17): Deprecation: local import search method found variable `imp15925.X` instead of nothing
+compilable/test15925.d(12): Deprecation: module `imp15925` from file compilable/imports/imp15925.d should be imported with 'import imp15925;'
+compilable/test15925.d(18): Deprecation: local import search method found variable `imp15925.X` instead of nothing
 ---
 */
 

test/compilable/test313f.d

@@ -1,7 +1,8 @@
 // REQUIRED_ARGS: -de
-import imports.f313;
+// EXTRA_SOURCES: imports/f313.d
+import foo.bar;
 
 void test()
 {
-    imports.f313.bug();
+    foo.bar.bug();
 }

test/compilable/test314.d

@@ -1,11 +1,12 @@
 // REQUIRED_ARGS: -de
+// EXTRA_SOURCES: imports/a314.d
 module imports.test314; // package imports
 
-import imports.a314;
+import imports.pkg.a314;
 
 void main()
 {
-    imports.a314.bug("This should work.\n");
+    imports.pkg.a314.bug("This should work.\n");
     renamed.bug("This should work.\n");
     bug("This should work.\n");
 }

test/d_do_test.d

@@ -459,24 +459,45 @@ bool collectExtraSources (in string input_dir, in string output_dir, in string[]
 }
 
 // compare output string to reference string, but ignore places
-// marked by $n$ that contain compiler generated unique numbers
+// marked by $n$ that contain compiler generated unique numbers and
+// also ignore dir seperators '/'
 bool compareOutput(string output, string refoutput)
 {
     import std.ascii : digits;
     import std.utf : byCodeUnit;
     for ( ; ; )
     {
+        bool skipDirSeparator = false;
         auto pos = refoutput.indexOf("$n$");
         if (pos < 0)
-            return refoutput == output;
+        {
+            pos = refoutput.indexOf("/");
+            if (pos < 0)
+            {
+                pos = refoutput.indexOf("\\");
+                if (pos < 0)
+                    return refoutput == output;
+            }
+            skipDirSeparator = true;
+        }
         if (output.length < pos)
             return false;
         if (refoutput[0..pos] != output[0..pos])
             return false;
-        refoutput = refoutput[pos + 3 ..$];
-        output = output[pos..$];
-        auto p = output.byCodeUnit.countUntil!(e => !digits.canFind(e));
-        output = output[p..$];
+        if (skipDirSeparator)
+        {
+            refoutput = refoutput[pos + 1 .. $];
+            if (output[pos] != '/' && output[pos] != '\\')
+                return false;
+            output    = output[pos + 1 .. $];
+        }
+        else
+        {
+            refoutput = refoutput[pos + 3 ..$];
+            output = output[pos..$];
+            auto p = output.byCodeUnit.countUntil!(e => !digits.canFind(e));
+            output = output[p..$];
+        }
     }
 }
 

test/fail_compilation/badimport.d

@@ -0,0 +1,10 @@
+/*
+EXTRA_SOURCES: imports/nomodname.d
+REQUIRED_ARGS: -Ifail_compilation -de
+PERMUTE_ARGS:
+TEST_OUTPUT:
+---
+fail_compilation/badimport.d(10): Error: module `nomodname` from file fail_compilation/imports/nomodname.d must be imported with 'import nomodname;'
+---
+*/
+import imports.nomodname;

test/fail_compilation/badimport2.d

@@ -0,0 +1,10 @@
+/*
+EXTRA_SOURCES: imports/incompletemodname.d
+REQUIRED_ARGS: -Ifail_compilation -de
+PERMUTE_ARGS:
+TEST_OUTPUT:
+---
+fail_compilation/badimport2.d(10): Error: module `incompletemodname` from file fail_compilation/imports/incompletemodname.d must be imported with 'import incompletemodname;'
+---
+*/
+import imports.incompletemodname;

test/fail_compilation/badimport3.d

@@ -0,0 +1,10 @@
+/*
+COMPILED_IMPORTS: imports/incompletemodname.d
+REQUIRED_ARGS: -Ifail_compilation
+PERMUTE_ARGS:
+TEST_OUTPUT:
+---
+fail_compilation/badimport3.d(10): Error: module `wrongpkg.wrongpkgname` from file fail_compilation/imports/wrongpkgname.d must be imported with 'import wrongpkg.wrongpkgname;'
+---
+*/
+import imports.wrongpkgname;

test/fail_compilation/badimport4.d

@@ -0,0 +1,10 @@
+/*
+EXRTRA_SOURCES: imports/wrong_mod_name.d
+REQUIRED_ARGS: -Ifail_compilation/imports
+PERMUTE_ARGS:
+TEST_OUTPUT:
+---
+fail_compilation/badimport4.d(10): Error: module `wrong_mod_name_bleh` from file fail_compilation/imports/wrong_mod_name.d must be imported with 'import wrong_mod_name_bleh;'
+---
+*/
+import wrong_mod_name;

test/fail_compilation/dip22b.d

@@ -1,8 +1,9 @@
 /*
+EXTRA_SOURCES: imports/dip22c.d
 REQUIRED_ARGS: -de
 TEST_OUTPUT:
 ---
-fail_compilation/dip22b.d(12): Deprecation: `pkg.dip22c.Foo` is not visible from module `dip22`
+fail_compilation/dip22b.d(13): Deprecation: `pkg.dip22c.Foo` is not visible from module `dip22`
 ---
 */
 module pkg.dip22;