Expose MARKDOWN_FILE constant

[IgnatBeresnev]

Part of #3112, since Dackka depends on it for creating a few custom tags.

The story with this constant is the following. There was a constant in the org.jetbrains.markdown library: org.intellij.markdown.MarkdownElementTypes.MARKDOWN_FILE.name whose value was MARKDOWN_FILE.

It was used as the name of CustomDocTag, and in a few other places.

Because Dokka used to expose PSI, kotlin-compiler and other internal libs as api dependencies, some downstream plugins (like Dackka) started using it as well, because they saw how Dokka used it and they just (allegedly) copied it.

When the analysis refactoring happened in #3034, all of these internal dependencies were no longer exposed to the user, so the code that was using org.intellij.markdown.MarkdownElementTypes.MARKDOWN_FILE.name couldn't be compiled anymore.

I looked at the use cases, but I couldn't figure out why it was needed in the first place. I mean, sure, it's used in CustomDocTag as the default name, so we should just rename it and call it CUSTOM_DOC_TAG_DEFAULT_NAME, right?

However, there are some dodgy checks like

dokka/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/transformers/pages/comments/DocTagToContentConverter.kt

Line 269 in 5bb1c8a

private fun CustomDocTag.isNonemptyFile() = name == MARKDOWN_ELEMENT_FILE_NAME && children.size > 1

and

dokka/dokka-subprojects/analysis-java-psi/src/main/kotlin/org/jetbrains/dokka/analysis/java/parsers/JavaDocCommentParser.kt

Line 203 in 5bb1c8a

val isFile = (tags.singleOrNull() as? CustomDocTag)?.name == MARKDOWN_ELEMENT_FILE_NAME

and it is mapped using this constant in

dokka/dokka-subprojects/analysis-markdown-jb/src/main/kotlin/org/jetbrains/dokka/analysis/markdown/jb/factories/DocTagsFromIElementFactory.kt

Line 61 in 5bb1c8a

MarkdownElementTypes.MARKDOWN_FILE -> CustomDocTag(children, params, MARKDOWN_ELEMENT_FILE_NAME)

So it makes me question whether CustomDocTag is used not only for custom tags, but also for something to do with markdown files?

To not make any rushed decisions that will need to be revisited later on anyway, I decided to expose this constant as it is, with an honest KDoc. We'll need to figure out what it was actually used for, and come up with an alternative, most likely when we start stabilizing the core API and its data model. Created an issue to keep track of it: #3365

[IgnatBeresnev]

This is the only meaningful change in this PR. Everything else just changes "MARKDOWN_FILE" literals to use this constant, so that we at least know the places where it is used.

[vmishenev]

Should it be marked by the InternalDokkaApi annotation?

[IgnatBeresnev]

The last paragraph in the KDoc has the answer :)

* This constant is not marked with [InternalDokkaApi] on purpose. Even though it is discouraged to use it,
* we understand that some existing code might depend on it, so once a replacement is provided,
* this constant should be deprecated with a message that will help users migrate to something stable.

[whyoleg]

Is it really a good idea to make java-psi and markdown analysis depend on kotlin-api?
For me it looks really strange...
May be it's better to move this declaration to core, in internal package? As we will anyway do something later with it?

[vmishenev]

To be honest, I don't understand why we can't figure out what it is actually used for in this PR. Instead of it, we expose the internal constant that plugins'll likely depend on. This constant's important as it's like a root name by default. Anyway we will force Dakka and other users to migrate to the new constant again.

We can try the new name (not MARKDOWN_FILE) here since Dakka should migrate to it anyway. We've already changed the package location. It's unnecessary to use the old name.
And we'll have a possibility to revisit it in #3365

However, there are some dodgy checks like

I think the names of these checks stem from the name of constant. They are local.

and it is mapped using this constant in

You can also see usages MarkdownElementTypes.MARKDOWN_FILE in the markdown artifact (or ask it's team). This is just the name of the root element.

[vmishenev]

Should it be marked by the InternalDokkaApi annotation?

[whyoleg]

A few results from the investigation on what is MARKDOWN_FILE.

Its usage was first introduced in this commit ec63795 of this PR #1473 to fix #1458.

The idea behind it is to have some kind of ROOT tag for documentation blocks to remove nesting P (paragraphs) inside P blocks - which is not supported by HTML. Before this PR if class has one line documentation, parser emit P(P(text)) , after PR: CustomTag(P(Text), name=MARKDOWN_FILE).
Also, after those changes looks like most (if not all) of the TagWrapper inheritors now have CustomTag as their root. name property there could be MARKDOWN_FILE (or sometime it could be just empty string) if it's unnamed tag, or name of NamedTagWrapper (this is how I understand the current logic, as this change happens later during merging in org.jetbrains.dokka.analysis.kotlin.descriptors.compiler.impl.moduledocs.ContextModuleAndPackageDocumentationReader#mergeDocumentationNodes) or name is equal to some markdown tag, that is not handled (not directly mapped to something in dokka), f.e. if markdown library will introduce some other new type, which isn't handled at our side.

Parsing documentation happens in org.jetbrains.dokka.analysis.markdown.jb.MarkdownParser#parseStringToDocNode and there, looks like for all cases it will return CustomDocTag with MARKDOWN_FILE or empty string. (it's even stated in the comment in PR: #1473 (comment))

So, in the end looks like it's just root tag, which for some reason is not the different class (like RootTag or GroupTag) - this needs further investigation.

BTW, I've added require(name == "MARKDOWN_FILE" || name == "") { "name=$name" } to init block of CustomDocTag and all tests passed :)

[whyoleg]

Filled #3404 based on comment above

[IgnatBeresnev]

I've rebased this PR onto master, and updated the KDoc to include the research results from above, along with a link to the issue: 64a9b09

I contemplated extracting this constant into core, naming it something like ROOT_TAG_NAME and describing the use case in more detail in KDocs, but in the end decided not to.

With a normal-looking name and inside core it looks like proper stable API, and limits the use case to the name and the KDoc description. While we've done good research, I still feel like some key detail might be missing due to the complexity of parsing, and for this reason it feels scary to improve this constant beyond necessary as part of this PR, as it might bring more unwanted questions / bugs / confusion / attention down the road.

I feel like it's best to address this properly as part of #3404, and have this PR serve as an enabler for Dackka and be a tiny incremental improvement.

I've also moved it out of the internal package and removed the recommendation to not use it, because we now know for a fact that it is required in some cases, whereas before we weren't sure. So it felt weird to have it in the internal package if it's something that plugins can't live without under certain rare conditions, but it doesn't seem to be used often enough to justify moving it to core - we'll be able to do it down the road with an easy replaceWith deprecation if it's needed.