Pathological import symbol shadowing

[H. S. Teoh]

On Wed, Nov 18, 2020 at 10:14:44AM +0000, FeepingCreature via Digitalmars-d wrote:
[...]
> This is why our style guide doesn't allow unqualified imports in
> functions.  The more I think about it, the more I suspect function
> local imports were just a mistake to begin with.
> 
> Bouncing back from module scope into import scope is just
> fundamentally bad for understandability. The set of relevant symbols
> goes from "all the ones imported" to "all the ones in the module" back
> to "all the ones imported" again, with imports shadowing local
> functions, which is exactly the opposite of what should happen.

There are several factors at work here.

Local imports are good in the sense that you minimize namespace
pollution: import the symbol only where you need them, not anywhere
else.

They are also good for encapsulation: cut and paste your function into
another module, and it Just Works.  Whereas if it depended on symbols
imported at the module level, you now have to copy-n-paste that import
into the new module.  And we all know what happens to that blob of
imports at the top of the file: it just keeps growing and growing, and
nobody dares delete it because you don't know what else might depend on
it. (Well, theoretically it's easy, just delete them one by one and put
back whichever causes a compile error. In practice, though, that's too
much work just for deleting a line of code, so nobody does it.)

The problems come from *unqualified* local imports, and on this point I
agree with you that unqualified local imports are generally a bad idea.
We've known of this since a long time ago, actually:

	void fun(string text) {
		import std.conv;
		writeln(text);
	}
	void main() {
		fun("hello world");
	}

This (used to) output a blank line, because `text` got hijacked by
std.conv.text.  IIRC somebody subsequently merged a DMD PR to change the
lookup rules to fix this case.  I thought that PR solved the problem
somehow, but as Walter said, it led to one special case after another.
And now here we are.


> Putting aside how Walter is completely correct that "import std" was a
> mistake, I'd go further and also remove unqualified module imports
> completely. There's a set of features that are good for writing but
> bad for reading. Generally as time goes on, people discover those
> features are bad ideas, but because writing comes before reading,
> everyone thinks they're a good idea at first, so languages are doomed
> to reinvent them.

I don't agree that `import std` is a mistake.  It's a wonderful shortcut
for one-off scripts and `echo ... | dmd -` one-liners.  Having to
explicitly spell out individual Phobos modules would completely kill the
latter usage because it becomes too onerous an effort for something
that's meant to be a one-time quick hack.

In fact, even in larger programs I think `import std` is a net plus,
because as somebody has already said, who really cares about that blob
of imports at the top of the file spelling out individual Phobos
modules?  Nobody reads it, and nobody cares except to satisfy the
compiler.  I can understand if you're importing some 3rd party library
and the compiler needs to be told where to find those symbols, but this
is the *standard* library, darnit.  It's supposed to be standard symbols
common across all D programs (more or less). Having to spell that out
every single time I use it is just Java-style needless boilerplate.

But I do agree that unqualified `import std` as a local import is a bad
idea, since it pulls in a whole truckload of symbols that, in all
likelihood, will clash with / shadow local symbols.  As I said, in
general unqualified local imports are bad news, because you don't know
what symbols might get pulled in; a local symbol can easily get hijacked
by a new symbol added to some 3rd party dependency.

Though I have to say, even despite the landmines, local `import std` is
useful for inserting temporary one-off debug code into otherwise
import-clean code. I often insert `import std; stderr.writefln("debug
info");` into strategic points in my code while debugging. It gives you
instant access to regular Phobos algorithms and functions without the
tedium of painstakingly spelling out every module you might need.  And
it's temporary and fits on one line, so it's just as easy to delete
afterwards without polluting the rest of the code.

So love it or hate it, `import std` *does* have its uses.

In this particular instance of problems caused by it, my opinion is
leaning towards std.curl.get being poorly-named. HTTP GET is a rather
narrow and specific function, as opposed to object.get for AA's which
are built into the language.  If it weren't for that ever-looming
spectre of breaking existing code, I'd say rename it to something else.
Like curlGet or something.


T

-- 
First Rule of History: History doesn't repeat itself -- historians merely repeat each other.