Merge pull request #5485 from MartinNowak/fix314

fix Issue 314 - static, renamed, and selective imports are always public

changelog.dd

@@ -6,6 +6,7 @@ $(COMMENT Pending changelog for 2.071. This will get copied to dlang.org and
 
 $(BUGSTITLE Compiler Changes,
     $(LI $(RELATIVE_LINK2 imports-313, Import access checks for fully qualified names were fixed.))
+    $(LI $(RELATIVE_LINK2 imports-314, Protection for selective and renamed imports were fixed.))
 )
 
 $(BUGSTITLE Language Changes,
@@ -30,6 +31,28 @@ $(BUGSTITLE Compiler Changes,
     $(P To ease updating existing code, the old behavior was retained but deprecated.
     )
     )
+
+    $(LI $(LNAME2 imports-314, Protection for selective and renamed imports were fixed.)
+
+    $(P It is no longer possible to use private selective or renamed imports in another module, e.g.
+        the following example will fail with `a.File is not visible from module b`.
+    )
+
+    ---
+    module a;
+    import std.stdio : File; // imports are private by default
+    ---
+    ---
+    module b;
+    import a;
+
+    File f; // File is private
+    ---
+
+    $(P To ease updating existing code, the old behavior was retained but deprecated.
+        This fix relies on the visibility changes introduced with $(RELATIVE_LINK2 dip22, DIP22).
+    )
+    )
 )
 
 $(BUGSTITLE Language Changes,

src/dimport.d

@@ -39,7 +39,7 @@ public:
     Identifier id;          // module Identifier
     Identifier aliasId;
     int isstatic;           // !=0 if static import
-    PROTKIND protection;
+    Prot protection;
 
     // Pairs of alias=name to bind into current namespace
     Identifiers names;
@@ -109,7 +109,7 @@ public:
 
     override Prot prot()
     {
-        return Prot(protection);
+        return protection;
     }
 
     // copy only syntax trees
@@ -216,11 +216,11 @@ public:
                         mod.deprecation(loc, "is deprecated");
                 }
                 mod.importAll(null);
+                if (sc.explicitProtection)
+                    protection = sc.protection;
                 if (!isstatic && !aliasId && !names.dim)
                 {
-                    if (sc.explicitProtection)
-                        protection = sc.protection.kind;
-                    sc.scopesym.importScope(mod, Prot(protection));
+                    sc.scopesym.importScope(mod, protection);
                 }
             }
         }
@@ -247,6 +247,9 @@ public:
             //printf("%s imports %s\n", sc.module.toChars(), mod.toChars());
             sc._module.aimports.push(mod);
 
+            if (sc.explicitProtection)
+                protection = sc.protection;
+
             if (!aliasId && !names.dim) // neither a selective nor a renamed import
             {
                 ScopeDsymbol scopesym;
@@ -260,9 +263,7 @@ public:
 
                 if (!isstatic)
                 {
-                    if (sc.explicitProtection)
-                        protection = sc.protection.kind;
-                    scopesym.importScope(mod, Prot(protection));
+                    scopesym.importScope(mod, protection);
                 }
 
                 // Mark the imported packages as accessible from the current
@@ -289,17 +290,7 @@ public:
                 sc._module.needmoduleinfo = 1;
             }
             sc = sc.push(mod);
-            /* BUG: Protection checks can't be enabled yet. The issue is
-             * that Dsymbol::search errors before overload resolution.
-             */
-            version (none)
-            {
-                sc.protection = protection;
-            }
-            else
-            {
-                sc.protection = Prot(PROTpublic);
-            }
+            sc.protection = protection;
             for (size_t i = 0; i < aliasdecls.dim; i++)
             {
                 AliasDeclaration ad = aliasdecls[i];
@@ -346,7 +337,7 @@ public:
             ob.writestring(") : ");
             // use protection instead of sc.protection because it couldn't be
             // resolved yet, see the comment above
-            protectionToBuffer(ob, Prot(protection));
+            protectionToBuffer(ob, protection);
             ob.writeByte(' ');
             if (isstatic)
             {
@@ -449,21 +440,9 @@ public:
                 importAll(sc);
 
             sc = sc.push(mod);
-            /* BUG: Protection checks can't be enabled yet. The issue is
-             * that Dsymbol::search errors before overload resolution.
-             */
-            version (none)
-            {
-                sc.protection = protection;
-            }
-            else
-            {
-                sc.protection = Prot(PROTpublic);
-            }
+            sc.protection = protection;
             foreach (ad; aliasdecls)
-            {
                 ad.setScope(sc);
-            }
             sc = sc.pop();
         }
     }

src/dsymbol.d

@@ -1368,7 +1368,12 @@ public:
                 if (!(flags & IgnoreErrors) && s.prot().kind == PROTprivate &&
                     !s.isOverloadable() && !s.parent.isTemplateMixin() && !s.parent.isNspace())
                 {
-                    if (!s.isImport())
+                    AliasDeclaration ad = void;
+                    // accessing private selective and renamed imports is
+                    // deprecated by restricting the symbol visibility
+                    if (s.isImport() || (ad = s.isAliasDeclaration()) !is null && ad._import !is null)
+                    {}
+                    else
                         error(loc, "%s %s is private", s.kind(), s.toPrettyChars());
                 }
                 //printf("\tfound in imports %s.%s\n", toChars(), s.toChars());

test/compilable/imports/a314.d

@@ -0,0 +1,5 @@
+module imports.pkg.a314; // sub package
+
+package(imports) static import imports.c314;
+package(imports) import renamed = imports.c314;
+package(imports) import imports.c314 : bug;

test/compilable/imports/c314.d

@@ -0,0 +1,4 @@
+module imports.c314;
+
+void bug(string s)
+{}

test/compilable/test314.d

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

test/fail_compilation/ice9865.d

@@ -5,5 +5,5 @@ fail_compilation/ice9865.d(8): Error: alias ice9865.Baz recursive alias declarat
 ---
 */
 
-import imports.ice9865b : Baz;
+public import imports.ice9865b : Baz;
 struct Foo { Baz f; }

test/fail_compilation/imports/a314.d

@@ -0,0 +1,5 @@
+module imports.a314;
+
+static import imports.c314;
+import renamed = imports.c314;
+import imports.c314 : bug;

test/fail_compilation/imports/b314.d

@@ -0,0 +1,4 @@
+module imports.b314;
+
+package import renamedpkg = imports.c314;
+package import imports.c314 : bugpkg=bug;

test/fail_compilation/imports/c314.d

@@ -0,0 +1,4 @@
+module imports.c314;
+
+void bug(string s)
+{}

test/fail_compilation/imports/ice9865b.d

@@ -1,2 +1,2 @@
-import ice9865;
+public import ice9865;
 class Bar { Foo foo; }

test/fail_compilation/test314.d

@@ -0,0 +1,25 @@
+/*
+REQUIRED_ARGS: -de
+TEST_OUTPUT:
+---
+fail_compilation/test314.d(3): Deprecation: imports.a314.renamed is not visible from module test314
+fail_compilation/test314.d(4): Deprecation: imports.a314.bug is not visible from module test314
+fail_compilation/test314.d(6): Deprecation: imports.b314.renamedpkg is not visible from module test314
+fail_compilation/test314.d(7): Deprecation: imports.b314.bugpkg is not visible from module test314
+---
+*/
+
+module test314;
+
+import imports.a314;
+import imports.b314;
+
+#line 1
+void main()
+{
+    renamed.bug("This should not work.\n");
+    bug("This should not work.\n");
+
+    renamedpkg.bug("This should not work.\n");
+    bugpkg("This should not work.\n");
+}