```private object MyDiagnostics: KtDiagnosticsCont...

# compiler

[min]

private object MyDiagnostics: KtDiagnosticsContainer() {
    val ARGUMENT_INVALID
        by error1<KtElement, FirClassSymbol<*>>()
    val PRIMARY_CONSTRUCTOR_MISSING
        by error1<KtElement, FirClassSymbol<*>>()
    val PRIMARY_CONSTRUCTOR_EXPOSED
        by error2<KtElement, FirClassSymbol<*>, FirConstructorSymbol>()

    override fun getRendererFactory(): BaseDiagnosticRendererFactory {
        return MyDiagnostics.GetRendererFactoryOutput
    }
    private object GetRendererFactoryOutput: BaseDiagnosticRendererFactory() {
        override val MAP: KtDiagnosticFactoryToRendererMap by KtDiagnosticFactoryToRendererMap("MyPlugin") { map ->
            map.put(
                MyDiagnostics.ARGUMENT_INVALID,
                "Invalid argument: `'0'` was expected to be an `annotation class`.",
                FirDiagnosticRenderers.RENDER_CLASS_OR_OBJECT(false) { id ->
                    id.shortClassName.identifier
                },
            )
            map.put(
                MyDiagnostics.PRIMARY_CONSTRUCTOR_MISSING,
                "Invalid class: `'0'` was expected to have a private primary constructor.",
                FirDiagnosticRenderers.DECLARATION_NAME,
            )
            map.put(
                MyDiagnostics.PRIMARY_CONSTRUCTOR_EXPOSED,
                "Invalid constructor: the `'0'` primary constructor is `'1'`, but was expected to be `private`.",
                FirDiagnosticRenderers.DECLARATION_NAME,
                TODO(),
            )
        }
    }
}

I have three questions about diagnostics:
1. Do I enclose interpolation argument indices in single quotes in messages (eg `"`'0'` was expected to be an `annotation class`"`) or how else do I format these messages?
2. How do I print the visibility of the FirConstructorSymbol in the MyDiagnostics.PRIMARY_CONSTRUCTOR_EXPOSED case?
3. Does my KtDiagnosticsContainer have to be registered somewhere like in ExtensionRegistrarContext#configurePlugin?

[Youssef Shoaib [MOD]]

3. There's a ExtensionRegistrarContext.registerDiagnosticContainers precisely for this purpose

[dmitriy.novozhilov]

How do I print the visibility of the FirConstructorSymbol in the MyDiagnostics.PRIMARY_CONSTRUCTOR_EXPOSED case?

You pass constructorSymbol.visibility as a diagnostic argument and use VISIBILITY renderer for this argument.

[min]

Hahaha, I was about to delete the question because I had just found the renderer! (There are a lot.) But thank you for the answer regardless. Much appreciated!

[min]

private object GetRendererFactoryOutput: BaseDiagnosticRendererFactory() {
    override val MAP: KtDiagnosticFactoryToRendererMap by KtDiagnosticFactoryToRendererMap("MyPlugin") { map ->
        map.put(
            MyDiagnostics.ARGUMENT_INVALID,
            "Invalid argument: `{0}` was expected to be an `annotation class`.",
            FirDiagnosticRenderers.RENDER_CLASS_OR_OBJECT(false) { id ->
                id.shortClassName.identifier
            },
        )
        map.put(
            MyDiagnostics.PRIMARY_CONSTRUCTOR_MISSING,
            "Invalid class: `{0}` was expected to declare a private primary constructor.",
            FirDiagnosticRenderers.DECLARATION_NAME,
        )
        map.put(
            MyDiagnostics.PRIMARY_CONSTRUCTOR_EXPOSED,
            "Invalid class: the `{1}` primary constructor of `{0}` was expected to be `private`",
            FirDiagnosticRenderers.DECLARATION_NAME,
            KtDiagnosticRenderers.VISIBILITY,
        )
    }
}

Is there a notable difference between FirDiagnosticRenderers and KtDiagnosticRenderers that I should be aware of?

[min]

Am I reinventing the wheel by doing

FirDiagnosticRenderers.RENDER_CLASS_OR_OBJECT(false) { id ->
    id.shortClassName.identifier
},

[min]

And would there be a better argument to pass to KtDiagnosticFactoryToRendererMap("MyPlugin") {...}?

[dmitriy.novozhilov]

Is there a notable difference between FirDiagnosticRenderers and KtDiagnosticRenderers that I should be aware of?

You could safely use both.
They are declared in different kotlin.git modules just in sake of code keeping the dependencies of modules clean.

Am I reinventing the wheel by doing

It's ok.

And would there be a better argument to pass to KtDiagnosticFactoryToRendererMap("MyPlugin") {...}?

It's fine too.

[min]

Thanks! I find it disorienting that
• Error detection is in MyFirAdditionalCheckersExtension
• And I have to associated each error with a message in a separate MyDiagnostics

private class MyAnnotation private constructor(
    val myAnnotatedClass: FirClassSymbol<*>,
    val myAnnotationArgument: ClassId,
    val primaryConstructor: FirConstructorSymbol,
) {
    companion object {
        fun parse(
            maybeAnnotatedClass: FirClassSymbol<*>,
            session: FirSession,
        ): Either<MyAnnotation, ParseError> {
            // ...
        }
    }

    sealed class ParseError {
        object ClassNotAnnotated: ParseError()
        object ArgumentMissing: ParseError()
        class ArgumentInvalid(
            val myAnnotatedClass: FirClassSymbol<*>,
        ): ParseError()
        class PrimaryConstructorMissing(
            val myAnnotatedClass: FirClassSymbol<*>,
        ): ParseError()
        class PrimaryConstructorExposed(
            val myAnnotatedClass: FirClassSymbol<*>,
            val primaryConstructor: FirConstructorSymbol,
        ): ParseError()
    }
}

class MyFirExtensionRegistrar: FirExtensionRegistrar() {
    override fun ExtensionRegistrarContext.configurePlugin() {
        (::MyFirAdditionalCheckersExtension).unaryPlus()
        (::MyFirDeclarationGenerationExtension).unaryPlus()
        this.registerDiagnosticContainers(
            MyFirAdditionalCheckersExtension.MyDiagnostics,
        )
    }
}

class MyFirAdditionalCheckersExtension(session: FirSession)
    : FirAdditionalCheckersExtension(session)
{
    override val declarationCheckers = object: DeclarationCheckers() {
        override val regularClassCheckers = setOf(object
            : FirRegularClassChecker(MppCheckerKind.Platform)
        {
            context(context: CheckerContext, reporter: DiagnosticReporter)
            override fun check(declaration: FirRegularClass) {
                when (val error = MyAnnotation.parse(declaration.symbol, context.session)
                    .asBar() ?: return
                ) {
                    is MyAnnotation.ParseError.ClassNotAnnotated -> return
                    is MyAnnotation.ParseError.ArgumentMissing -> return
                    is MyAnnotation.ParseError.ArgumentInvalid -> reporter.reportOn(
                        error.myAnnotatedClass.source,
                        MyDiagnostics.ARGUMENT_INVALID,
                        error.myAnnotatedClass,
                        context,
                    )
                    is MyAnnotation.ParseError.PrimaryConstructorMissing -> reporter.reportOn(
                        error.myAnnotatedClass.source,
                        MyDiagnostics.PRIMARY_CONSTRUCTOR_MISSING,
                        error.myAnnotatedClass,
                        context,
                    )
                    is MyAnnotation.ParseError.PrimaryConstructorExposed -> reporter.reportOn(
                        error.primaryConstructor.source,
                        MyDiagnostics.PRIMARY_CONSTRUCTOR_EXPOSED,
                        error.myAnnotatedClass,
                        error.primaryConstructor.visibility,
                        context,
                    )
                }
            }
        })
    }

    object MyDiagnostics: KtDiagnosticsContainer() {
        val ARGUMENT_INVALID
            by error1<KtElement, FirClassSymbol<*>>()
        val PRIMARY_CONSTRUCTOR_MISSING
            by error1<KtElement, FirClassSymbol<*>>()
        val PRIMARY_CONSTRUCTOR_EXPOSED
            by error2<KtElement, FirClassSymbol<*>, Visibility>()

        override fun getRendererFactory(): BaseDiagnosticRendererFactory {
            return MyDiagnostics.GetRendererFactoryOutput
        }
        private object GetRendererFactoryOutput: BaseDiagnosticRendererFactory() {
            override val MAP: KtDiagnosticFactoryToRendererMap by KtDiagnosticFactoryToRendererMap("MyPlugin") { map ->
                map.put(
                    MyDiagnostics.ARGUMENT_INVALID,
                    "Invalid argument: `{0}` was expected to be an `annotation class`.",
                    FirDiagnosticRenderers.RENDER_CLASS_OR_OBJECT(quoted = false) { id ->
                        id.shortClassName.identifier
                    },
                )
                map.put(
                    MyDiagnostics.PRIMARY_CONSTRUCTOR_MISSING,
                    "Invalid class: `{0}` was expected to declare a private primary constructor.",
                    FirDiagnosticRenderers.DECLARATION_NAME,
                )
                map.put(
                    MyDiagnostics.PRIMARY_CONSTRUCTOR_EXPOSED,
                    "Invalid class: the `{1}` primary constructor of `{0}` was expected to be `private`.",
                    FirDiagnosticRenderers.DECLARATION_NAME,
                    KtDiagnosticRenderers.VISIBILITY,
                )
            }
        }
    }
}

Is there an idiomatic way to consolidate that information for improved local reasoning? I want information to be spread out between fewer classes.

[dmitriy.novozhilov]

No, it's intended to have a lot of classes here. One for diagnostics, one for messages, one class (object) for each checker, three checker containers (expression/declaration/type). And one for extension if we're talking about plugins.

[min]

(::MyFirAdditionalCheckersExtension)._unaryPlus_() in ExtensionRegistrarContext#configurePlugin must be accompanied by a call to ExtensionRegistrarContext#registerDiagnosticContainers which passes the KtDiagnosticsContainer associated with the reports produced by the checkers extension.

override fun ExtensionRegistrarContext.configurePlugin() {
    (::MyFirAdditionalCheckersExtension).unaryPlus()
    this.registerDiagnosticContainers(
        MyFirAdditionalCheckersExtension.MyDiagnostics,
    )
}

I find this brittle since updating my plugin to add or remove a checkers extension could break it if the registrar is left with an incompatible set of diagnostic containers. If an extension is coupled with multiple diagnostic containers and shares some with other extensions, it becomes even more complex to manually track the validity of the registered set of diagnostic containers.

class MyFirAdditionalCheckersExtension(session: FirSession)
    : FirAdditionalCheckersExtension(session)
{
    // Other overrides

    override fun onRegister(context: ExtensionRegistrarContext) {
        context.registerDiagnosticContainers(
            MyFirAdditionalCheckersExtension.MyDiagnostics,
        )
    }
}

Is there a way to mitigate this risk such as some kind of callback that runs on register that’d let me localise the information on the necessary diagnostic containers to each FirAdditionalCheckersExtension subclass?

[Youssef Shoaib [MOD]]

I think the order of operations doesn't work out. FirAdditionalCheckersExtension needs a FirSession, while diagnostic containers are registered before you receive such a session

[min]

Well, the onRegister method is only a hypothetical example. The actual solution doesn’t have to look exactly like it at all, for as long as the risk is mitigated or (preferably) eliminated!

[dmitriy.novozhilov]

It could be improved, yeah. Feel free to create a feature request.

But generally it's expected to have at most 1 checkers extension and 1 diagnostics container per plugin.

[Youssef Shoaib [MOD]]

You could wrap them in one if you really want: (untested)

interface FirCheckersMegaFactory {
  val diagnosticsContainers: List<DiagnosticSContainer>
  fun create(session: FirSession): FirAdditionalCheckersExtension
}
context(c: ExtensionRegistrarContext)
operator fun 
FirCheckersMegaFactory.unaryPlus() {
  +::create
  registerDiagnosticsContainers(diagnosticsContainers)
}
class MyCheckersExtension(...): ... {
  ...
  companion object: FirCheckersMegaFactory {
    override fun create(session: FirSession) = MyCheckersExtension(...)
    override val diagnosticsContainers = listOf(MyDiagnostics)
  }
}

[min]

Sorry, I didn’t mean to criticise. I was just confused because that’s not what the API seems (to me) to suggest at all. Thank you for explaining the expectation.