scribe icon indicating copy to clipboard operation
scribe copied to clipboard

Compilation error after updating from 3.16.0 to 3.16.1

Open tgodzik opened this issue 9 months ago • 5 comments

I just noticed that after the recent update, the Metals codebase is no longer compiling, which is unexpected in a patch release:

[error] /Users/runner/work/metals/metals/metals/src/main/scala/scala/meta/internal/metals/logging/MetalsLogger.scala:180:9: object creation impossible.
[error] Missing implementation for member of trait LoggerSupport:
[error]   def log(record: => scribe.LogRecord): Unit = ??? // implements `def log(record: => scribe.LogRecord): F`
[error]     new LoggerSupport[Unit] {
[error]         ^
[error] /Users/runner/work/metals/metals/metals/src/main/scala/scala/meta/internal/metals/logging/MetalsLogger.scala:181:20: method log overrides nothing.
[error] Note: the super classes of <$anon: scribe.LoggerSupport[Unit]> contain the following, non final members named log:
[error] def log(level: scribe.Level, mdc: scribe.mdc.MDC, features: scribe.LogFeature*)(implicit pkg: sourcecode.Pkg, implicit fileName: sourcecode.FileName, implicit name: sourcecode.Name, implicit line: sourcecode.Line): Unit
[error] def log(record: => scribe.LogRecord): Unit
[error]       override def log(record: LogRecord): Unit = ()
[error]                    ^
[warn] two warnings found
[error] two errors found

The snippet is just this:

  def silent: LoggerSupport[Unit] =
    new LoggerSupport[Unit] {
      override def log(record: LogRecord): Unit = ()
    }

Any idea what changed? It must have been some library bump 🤔

Thanks for the great library!

tgodzik avatar Apr 07 '25 06:04 tgodzik

Looks like the issue is https://github.com/outr/scribe/commit/9d55310fa13ec01e5e1685bd39b86e90e375e6cb 🤔

Should be easy to fix, but wonder if intended.

tgodzik avatar Apr 07 '25 07:04 tgodzik

Yeah, I realized after the patch release that this had more unintended consequences than I realized. The error actually explicitly outlines the problem. The method log now takes record: => LogRecord instead of record: LogRecord. It was intended as a minor change when used with effectless implementations to avoid the timestamp being recorded at initial call and only when the task is executed.

darkfrog26 avatar Apr 07 '25 13:04 darkfrog26

Sorry about that. It should have occurred to me that this would break things.

darkfrog26 avatar Apr 07 '25 13:04 darkfrog26

No worries, I just felt it's worth mentioning. I got it fixed in metals already

tgodzik avatar Apr 07 '25 13:04 tgodzik

I appreciate you reaching out. Also, if there's ever any special functionality you guys need in Metals please let me know.

darkfrog26 avatar Apr 07 '25 14:04 darkfrog26