ScalablyTyped/Converter

Error overriding method .. needs `override` modifier

FabioPinheiro opened this issue · 1 comments

I have a lot of errors of the type Error overriding method .... needs 'override' modifier

When I tried to compile the npm package "@material/web" -> "0.1.0-alpha.2"

I'm using scala 3.2.2-RC2; scalajs 1.12.0; sbt-converter 1.0.0-beta40

I also had the impression that was a flag to disable that check that requires the override modifier.

I can try to minimize the project, try other scala versions and try to investigate a bit. But maybe @oyvindberg you know where to start or if I'm doing anything wrong.

[error] java.lang.RuntimeException: FromFolder(InFolder(/home/fabio/workspace/ScalaDID/webapp/target/scala-3.2.2-RC2/scalajs-bundler/main/node_modules/@material/web),TsIdentLibraryScoped(material,web)) -> Right(Compilation failed:   method render of type (): typings.litHtml.mod.TemplateResult[typings.litHtml.mod.ResultType] needs `override` modifier, Error, Optional[-- [E164] Declaration Error: /home/fabio/workspace/ScalaDID/webapp/target/streams/_global/stImport/_global/streams/sources/m/material__web/src/main/scala/typings/materialWeb/actionChipMod.scala:20:24 )
[error] 	at scala.sys.package$.error(package.scala:30)
[error] 	at org.scalablytyped.converter.plugin.ScalablyTypedConverterPlugin$.$anonfun$stImportTask$6(ScalablyTypedConverterPlugin.scala:81)
[error] 	at scala.Function1.$anonfun$compose$1(Function1.scala:49)
[error] 	at sbt.internal.util.$tilde$greater.$anonfun$$u2219$1(TypeFunctions.scala:62)
[error] 	at sbt.std.Transform$$anon$4.work(Transform.scala:68)
[error] 	at sbt.Execute.$anonfun$submit$2(Execute.scala:282)
[error] 	at sbt.internal.util.ErrorHandling$.wideConvert(ErrorHandling.scala:23)
[error] 	at sbt.Execute.work(Execute.scala:291)
[error] 	at sbt.Execute.$anonfun$submit$1(Execute.scala:282)
[error] 	at sbt.ConcurrentRestrictions$$anon$4.$anonfun$submitValid$1(ConcurrentRestrictions.scala:265)
[error] 	at sbt.CompletionService$$anon$2.call(CompletionService.scala:64)
[error] 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[error] 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
[error] 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[error] 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
[error] 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
[error] 	at java.base/java.lang.Thread.run(Thread.java:829)

Hey there. I had a quick look.

TLDR: compile it in library developer mode and add the overrides and/or remove members manually. check in and publish the code.

details

There is a pipeline of transformations which massage class hierarchies into something scalac may consider accepting. It looks like this:

        new S.RemoveMultipleInheritance(parentResolver()).visitPackageTree(scope),
        new S.CombineOverloads(erasure())
          .visitPackageTree(scope), //must have stable types, so FakeLiterals run before
        new S.FilterMemberOverrides(erasure(), parentResolver()).visitPackageTree(scope), //
        new S.InferMemberOverrides(erasure(), parentResolver())
          .visitPackageTree(scope), //runs in phase after FilterMemberOverrides
        new S.CompleteClass(erasure(), parentResolver(), scalaVersion)
          .visitPackageTree(scope), //after FilterMemberOverrides

This all works reasonably well, but there is an inherent weakness in how the transformations in the pipeline are run: when ST works on a class and inspects parent classes, those parent classes have not been fully transformed yet if they are in the same typescript library. I've meant to refactor this forever, but haven't had the time.

What happens here is this scenario:

The class ActionElement has a definition for this field

    /* standard dom */
    /* CompleteClass */
    var ariaAtomic: String | Null = js.native

which it inherits from the class ReactiveElement:

    /* standard dom */
    /* CompleteClass */
    var ariaAtomic: String | Null = js.native

both inherits the var from typings.std.ARIAMixin in abstract form:

var ariaAtomic: String | Null

The main issue that CompleteClass doesn't see the addition to ReactiveElement when it adds it to `ActionElement. I think there are other, smaller issues in here too, but that looks like the main thing.

I won't patch this up now, but will fix it along with that refactoring sometime down the road. manual fixup is your best shot for now.