Add Docs.undocumented_names

[jariji]

Fixes #51174

Cross-ref: #52139

[jariji]

@stevengj ready for review.

[stevengj]

I suspect we don’t want a function that throws an error here. We probably just want a function that returns a sorted list of undocumented symbols. Then people can just add tests like

@test Docs.undocumented_names(MyMod) == [:known1, :known2]
@test_broken Docs.undocumented_names(MyMod2) == []

[jariji]

Expression: Docs.undocumented_names(_ModuleWithSomeDocumentedNames; all = true) == [:g]
   Evaluated: [Symbol("##meta#47"), Symbol("#eval"), Symbol("#f"), Symbol("#g"), Symbol("#include"), :eval, :g, :include] == [:g]

What should happen here? @stevengj

[stevengj]

Symbol("##meta#47"), Symbol("#eval"), Symbol("#f"), Symbol("#g"), Symbol("#include") are internal names generated by Julia's compiler.

For now, if you want to check the all=true case, I would filter the symbols for !Base.isidentifier to avoid involving compiler internals in the test. In the future, we may want to add an isidentifier::Bool=false option to names as well as this function in order to filter out non-identifier symbols by default.

[jariji]

idk what that failure is.

[jariji]

This is ready for merge.

[LilithHafner]

Sorry I'm a little late here; I'm not sure implementing this function in Base.Docs is a good idea. It seems like the kind of functionality that is used in development and testing but not in ordinary usage. Consequently, I think it belongs in Test or Aqua where it can be implemented in terms of Base's existing public API (thanks to @jariji's #52139) My opinion here is pretty weak, and I'd be open to possibly keeping it here, but I do think this should be discussed before we commit to adding this public API to Base.Docs

Regardless of the above, we should fix the docstring which currently misrepresents the behavior on undocumetned public but unexported symbols. (IIUC docstring: they are not returned by default; reality: they are returned by default)

[LilithHafner]

Clarification: IMO having this functionality is definitely a good idea, the question is where to have it.

[stevengj]

If it goes anywhere else, it should be in Test (not Aqua) so that it can be used to test Base modules and stdlibs. (That was my initial suggestion from #51174.) I don't remember why we didn't discuss it here, sorry.

[stevengj]

I think the argument to put it in Docs was due to @Seelengrab in #51174 (comment) … it was basically about adding a dependency on Docs to Test, when this function doesn't actually use anything in Test:

Why should Test depend on Docs for something that is purely Docs functionality? Either having that in Docs or in an extension of Test weakly depending on Docs would be fine IMO.

(Though since the Docs module is in Base I don't really get this argument?)

If we put it in Test, we might want to simply export it.

[Seelengrab]

You're right, having Docs be a part of Base means the dependency is a non-issue code-loading wise.

Still, the thought was that undocumented_names is not inherently related to testing functionality; there's a lot of things you could do with this (e.g. mark those names specially in Documenter, Pluto, some other tool) that aren't related to unit testing at all. Forcing that code to depend on Test in addition to Docs when the rest of the functionality of Test is not needed seems odd to me.

[stevengj]

I just realized that this filters out macro symbols @foo, which we don't want to exclude. Probably should be

str = string(sym)
!hasdoc(mod, sym) && (Base.isidentifier(sym) || (startswith(str, "@") && Base.isidentifier(str[2:end])))

[DilumAluthge]

Similar to the argument that I make in #52724, I think that we should rename Docs.undocumented_names to something a bit more specific and less ambiguous, to clarify that we are talking about names without docstrings.

I've opened #52726, which proposes renaming it to Docs.names_without_docstring, but I'm open to other names as well.

[LilithHafner]

From the dregs of triage (Lilith, Jeff, Jar) after most folks left:

Base.Docs is a fine place for this functionality to go. It's nontrivial to get right, so worth implementing, and it's needed by Base, so worth putting in Base/Docs/Test. It can be used even in the absence of testing, so shouldn't be in Test.

undocumented_names is a reasonable name. The only thing that "documented" could mean, from Base.Docs's perspective, is "has a docstring" because Base.Docs doesn't know about the manual or any Documenter.jl stuff.

_is_hidden_name(s::Symbol) = !isempty(string(x)) && string(x)[1] == '#'

And use this semantic for Docs.undocumented_names(mod; private=false):
For private=false, default: filter(x -> !Base.Docs.hasdoc(mod, x) && !_is_hidden_name(x) && Base.ispublic(mod, x), names(mod, all=true))
For private=true: filter(x -> !Base.Docs.hasdoc(mod, x) && !_is_hidden_name(x), names(mod, all=true))

Following the pattern of names which has keywords that default to false and setting them to true adds more names to be returned.