Merge pull request #147 from dayllenger/public-imports

Fix public selective imports
merged-on-behalf-of: Jan Jurzitza <[email protected]>

src/dsymbol/conversion/first.d

@@ -540,7 +540,7 @@ final class FirstPass : ASTVisitor
 				SemanticSymbol* renameSymbol = allocateSemanticSymbol(
 					internString(single.rename.text), CompletionKind.aliasName,
 					modulePath);
-				renameSymbol.acSymbol.skipOver = protection.current != tok!"public";
+				renameSymbol.acSymbol.skipOver = protection.currentForImport != tok!"public";
 				renameSymbol.acSymbol.type = importSymbol.acSymbol;
 				renameSymbol.acSymbol.ownType = true;
 				renameSymbol.addChild(importSymbol, true);
@@ -584,7 +584,7 @@ final class FirstPass : ASTVisitor
 
 			importSymbol.acSymbol.qualifier = SymbolQualifier.selectiveImport;
 			importSymbol.typeLookups.insert(lookup);
-			importSymbol.acSymbol.skipOver = protection.current != tok!"public";
+			importSymbol.acSymbol.skipOver = protection.currentForImport != tok!"public";
 			currentSymbol.addChild(importSymbol, true);
 			currentScope.addSymbol(importSymbol.acSymbol, false);
 		}

src/dsymbol/conversion/second.d

@@ -103,14 +103,14 @@ void resolveImport(DSymbol* acSymbol, ref UnrolledList!(TypeLookup*, Mallocator,
 	ref ModuleCache cache)
 in
 {
-	assert (acSymbol.kind == CompletionKind.importSymbol);
+	assert(acSymbol.kind == CompletionKind.importSymbol);
+	assert(acSymbol.symbolFile !is null);
 }
 body
 {
+	DSymbol* moduleSymbol = cache.cacheModule(acSymbol.symbolFile);
 	if (acSymbol.qualifier == SymbolQualifier.selectiveImport)
 	{
-		assert(acSymbol.symbolFile !is null);
-		DSymbol* moduleSymbol = cache.cacheModule(acSymbol.symbolFile);
 		if (moduleSymbol is null)
 		{
 		tryAgain:
@@ -128,7 +128,7 @@ body
 
 			istring symbolName = typeLookups.front.breadcrumbs.front;
 			DSymbol* selected = moduleSymbol.getFirstPartNamed(symbolName);
-			if (acSymbol is null)
+			if (selected is null)
 				goto tryAgain;
 			acSymbol.type = selected;
 			acSymbol.ownType = false;
@@ -144,8 +144,6 @@ body
 	}
 	else
 	{
-		assert(acSymbol.symbolFile !is null);
-		DSymbol* moduleSymbol = cache.cacheModule(acSymbol.symbolFile);
 		if (moduleSymbol is null)
 		{
 			DeferredSymbol* deferred = Mallocator.instance.make!DeferredSymbol(acSymbol);

src/dsymbol/scope_.d

@@ -89,9 +89,8 @@ struct Scope
 		import std.algorithm.iteration : map;
 
 		auto s = getScopeByCursor(cursorPosition);
-
 		if (s is null)
-			return [];
+			return null;
 
 		UnrolledList!(DSymbol*) retVal;
 		Scope* sc = s;
@@ -112,7 +111,7 @@ struct Scope
 						foreach (i; item.ptr.type.opSlice())
 							retVal.insert(i);
 					}
-					else if (item.ptr !is null)
+					else
 						retVal.insert(item.ptr.type);
 				}
 				else

src/dsymbol/symbol.d

@@ -225,45 +225,62 @@ struct DSymbol
 			return;
 		visited.insert(cast(size_t) &this);
 
-		DSymbol p = DSymbol(IMPORT_SYMBOL_NAME);
-		if (qualifier == SymbolQualifier.selectiveImport && type !is null
-				&& (name is null || type.name == name))
+		if (name is null)
 		{
-			app.put(cast(DSymbol*) type);
-			if (onlyOne)
-				return;
+			foreach (part; parts[].filter!(a => a.name != IMPORT_SYMBOL_NAME))
+			{
+				app.put(cast(DSymbol*) part);
+				if (onlyOne)
+					return;
+			}
+			DSymbol p = DSymbol(IMPORT_SYMBOL_NAME);
+			foreach (im; parts.equalRange(SymbolOwnership(&p)))
+			{
+				if (im.type !is null && !im.skipOver)
+				{
+					if (im.qualifier == SymbolQualifier.selectiveImport)
+					{
+						app.put(cast(DSymbol*) im.type);
+						if (onlyOne)
+							return;
+					}
+					else
+						im.type.getParts(name, app, visited, onlyOne);
+				}
+			}
 		}
 		else
 		{
-			if (name is null)
+			DSymbol s = DSymbol(name);
+			foreach (part; parts.equalRange(SymbolOwnership(&s)))
 			{
-				foreach (part; parts[].filter!(a => a.name != IMPORT_SYMBOL_NAME))
-				{
-					app.put(cast(DSymbol*) part);
-					if (onlyOne)
-						return;
-				}
-				foreach (im; parts.equalRange(SymbolOwnership(&p)))
-					if (im.type !is null && !im.skipOver)
-						im.type.getParts(name, app, visited, onlyOne);
+				app.put(cast(DSymbol*) part);
+				if (onlyOne)
+					return;
 			}
-			else
+			if (name == CONSTRUCTOR_SYMBOL_NAME ||
+				name == DESTRUCTOR_SYMBOL_NAME ||
+				name == UNITTEST_SYMBOL_NAME ||
+				name == THIS_SYMBOL_NAME)
+				return;	// these symbols should not be imported
+
+			DSymbol p = DSymbol(IMPORT_SYMBOL_NAME);
+			foreach (im; parts.equalRange(SymbolOwnership(&p)))
 			{
-				DSymbol s = DSymbol(name);
-				foreach (part; parts.equalRange(SymbolOwnership(&s)))
+				if (im.type !is null && !im.skipOver)
 				{
-					app.put(cast(DSymbol*) part);
-					if (onlyOne)
-						return;
-				}
-				if (name == CONSTRUCTOR_SYMBOL_NAME ||
-					name == DESTRUCTOR_SYMBOL_NAME ||
-					name == UNITTEST_SYMBOL_NAME ||
-					name == THIS_SYMBOL_NAME)
-					return;	// these symbols should not be imported
-				foreach (im; parts.equalRange(SymbolOwnership(&p)))
-					if (im.type !is null && !im.skipOver)
+					if (im.qualifier == SymbolQualifier.selectiveImport)
+					{
+						if (im.type.name == name)
+						{
+							app.put(cast(DSymbol*) im.type);
+							if (onlyOne)
+								return;
+						}
+					}
+					else
 						im.type.getParts(name, app, visited, onlyOne);
+				}
 			}
 		}
 	}
@@ -396,7 +413,7 @@ struct DSymbol
 		ubyte, "", 5));
 	// dfmt on
 
-	/// Protection level for this symobol
+	/// Protection level for this symbol
 	IdType protection;
 
 }

src/dsymbol/tests.d

@@ -352,6 +352,81 @@ unittest
 	assert(T2.kind == CompletionKind.variadicTmpParam);
 }
 
+unittest
+{
+	writeln("Running public import tests...");
+
+	const dir = buildPath(tempDir(), "dsymbol");
+	const fnameA = buildPath(dir, "a.d");
+	const fnameB = buildPath(dir, "b.d");
+	const fnameC = buildPath(dir, "c.d");
+	const fnameD = buildPath(dir, "d.d");
+	const srcA = q{ int x; int w; };
+	const srcB = q{ float y; private float z; };
+	const srcC = q{ public { import a : x; import b; } import a : w; long t; };
+	const srcD = q{ public import c; };
+	// A simpler diagram:
+	// a = x w
+	// b = y [z]
+	// c = t + (x y) [w]
+	// d = (t x y)
+
+	mkdir(dir);
+	write(fnameA, srcA);
+	write(fnameB, srcB);
+	write(fnameC, srcC);
+	write(fnameD, srcD);
+	scope (exit)
+	{
+		remove(fnameA);
+		remove(fnameB);
+		remove(fnameC);
+		remove(fnameD);
+		rmdir(dir);
+	}
+
+	ModuleCache cache = ModuleCache(theAllocator);
+	cache.addImportPaths([dir]);
+
+	const a = cache.getModuleSymbol(istring(fnameA));
+	const b = cache.getModuleSymbol(istring(fnameB));
+	const c = cache.getModuleSymbol(istring(fnameC));
+	const d = cache.getModuleSymbol(istring(fnameD));
+	const ax = a.getFirstPartNamed(istring("x"));
+	const aw = a.getFirstPartNamed(istring("w"));
+	assert(ax);
+	assert(aw);
+	assert(ax.type && ax.type.name == "int");
+	assert(aw.type && aw.type.name == "int");
+	const by = b.getFirstPartNamed(istring("y"));
+	const bz = b.getFirstPartNamed(istring("z"));
+	assert(by);
+	assert(bz);
+	assert(by.type && by.type.name == "float");
+	assert(bz.type && bz.type.name == "float");
+	const ct = c.getFirstPartNamed(istring("t"));
+	const cw = c.getFirstPartNamed(istring("w"));
+	const cx = c.getFirstPartNamed(istring("x"));
+	const cy = c.getFirstPartNamed(istring("y"));
+	const cz = c.getFirstPartNamed(istring("z"));
+	assert(ct);
+	assert(ct.type && ct.type.name == "long");
+	assert(cw is null); // skipOver is true
+	assert(cx is ax);
+	assert(cy is by);
+	assert(cz is bz); // should not be there, but it is handled by DCD
+	const dt = d.getFirstPartNamed(istring("t"));
+	const dw = d.getFirstPartNamed(istring("w"));
+	const dx = d.getFirstPartNamed(istring("x"));
+	const dy = d.getFirstPartNamed(istring("y"));
+	const dz = d.getFirstPartNamed(istring("z"));
+	assert(dt is ct);
+	assert(dw is null);
+	assert(dx is cx);
+	assert(dy is cy);
+	assert(dz is cz);
+}
+
 unittest
 {
 	ModuleCache cache = ModuleCache(theAllocator);