It looks like the logic to skip generation of copy methods for abstract classes is flawed.
When making the hierarchy deeper:
open class TimeInterval {}
abstract class Day extends TimeInterval {
...
it even fails to build with "Cannot create an instance of an abstract class." (apart from 'copy' overrides nothing.). This case should also be considered when fixing the implementation of copy.
Marking Day as open instead of abstract can be used to work around this issue.
Originally created by @juriad on GitHub (Jul 6, 2024).
Try generating Kotlin classes for this template:
```
abstract class Day {
day: Int
month: Int
year: Int
}
class Weekend extends Day {}
class Workday extends Day {}
days: Listing<Day>
```
The classes `Weekend` and `Workday` contain copy methods:
```
override fun copy(
day: Long,
month: Long,
year: Long
): Workday = Workday(day, month, year)
```
but the class `Day` which they extend:
```
class Workday(
day: Long,
month: Long,
year: Long
) : Day(day, month, year) {
```
does not define `copy` method, which leads to a compilation error:
```
e: file:///home/whoever/whatever/app/build/generated/pkl/template/kotlin/Calendar.kt:52:5 'copy' overrides nothing.
```
It looks like the logic to [skip generation](https://github.com/apple/pkl/blob/5cc2ea2d003a1e2c6e856363684e2aa085ed38a9/pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/KotlinCodeGenerator.kt#L282) of `copy` methods for abstract classes is flawed.
When making the hierarchy deeper:
```
open class TimeInterval {}
abstract class Day extends TimeInterval {
...
```
it even fails to build with "Cannot create an instance of an abstract class." (apart from `'copy' overrides nothing.`). This case should also be considered when fixing the implementation of `copy`.
Marking `Day` as `open` instead of `abstract` can be used to work around this issue.
Doesn't a copy method require that the class is instantiable? I would not expect a copy method on an abstract class.
The spurious override does seem to be a bug (but I'd say a separate one).
@holzensp commented on GitHub (Jul 15, 2024):
Doesn't a `copy` method require that the class is instantiable? I would not expect a `copy` method on an `abstract` class.
The spurious `override` does seem to be a bug (but I'd say a separate one).
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Originally created by @juriad on GitHub (Jul 6, 2024).
Try generating Kotlin classes for this template:
The classes
WeekendandWorkdaycontain copy methods:but the class
Daywhich they extend:does not define
copymethod, which leads to a compilation error:It looks like the logic to skip generation of
copymethods for abstract classes is flawed.When making the hierarchy deeper:
it even fails to build with "Cannot create an instance of an abstract class." (apart from
'copy' overrides nothing.). This case should also be considered when fixing the implementation ofcopy.Marking
Dayasopeninstead ofabstractcan be used to work around this issue.@holzensp commented on GitHub (Jul 15, 2024):
Doesn't a
copymethod require that the class is instantiable? I would not expect acopymethod on anabstractclass.The spurious
overridedoes seem to be a bug (but I'd say a separate one).@odenix commented on GitHub (Oct 17, 2024):
I’ll send a PR that fixes both of the bugs reported here.