google / closure-stylesheets

A CSS+ transpiler that lints, optimizes, and I18n-izes
Apache License 2.0
314 stars 65 forks source link

Crash when using non-empty input-renaming-map #124

Open Yannic opened 7 years ago

Yannic commented 7 years ago

Closure Stylesheets crashes with the following exception when using an non-empty input-renaming-map. The format of the renaming map does not seem to matter, it only must not be empty.

Exception in thread "main" java.lang.ClassCastException: com.google.common.css.IdentitySubstitutionMap cannot be cast to com.google.common.css.SubstitutionMap$Initializable
    at com.google.common.css.RecordingSubstitutionMap.initializeWithMappings(RecordingSubstitutionMap.java:91)
    at com.google.common.css.compiler.passes.PassRunner.createSubstitutionMap(PassRunner.java:213)
    at com.google.common.css.compiler.passes.PassRunner.<init>(PassRunner.java:49)
    at com.google.common.css.compiler.commandline.DefaultCommandLineCompiler.<init>(DefaultCommandLineCompiler.java:76)
    at com.google.common.css.compiler.commandline.ClosureCommandLineCompiler.<init>(ClosureCommandLineCompiler.java:65)
    at com.google.common.css.compiler.commandline.ClosureCommandLineCompiler.executeJob(ClosureCommandLineCompiler.java:371)
    at com.google.common.css.compiler.commandline.ClosureCommandLineCompiler.main(ClosureCommandLineCompiler.java:440)

Repo: java -jar closure-stylesheets-1.5.0-SNAPSHOT-jar-with-dependencies.jar --input-renaming-map renaming.json some.css

renaming.json

{
  "foo": "bar"
}

some.css Content does not matter, can be empty.

elipoultorak commented 5 years ago

Any reason this wasn't merged yet? The problem still exists, and @Yannic's commit fixes it.

sgammon commented 5 years ago

@elipoultorak it does fix it but if memory serves, it just disables the input renaming map? or did it work for you? if so then yeah why isn't this merged lol

Yannic commented 5 years ago

TBH, I don't remember what problem I was trying to solve when I initially run into this crash, and I haven't seen it for the last two years. IIRC, #125 worked but wasn't merged because the "fix" just hides the issue that the map isn't initialized where that's supposed to happen.

elipoultorak commented 5 years ago

I just tried it, and for some reason, it does rename it properly with the fix. I used a CLOSURE_COMPILED format instead of json, so I don't know if it would work with json. But when I use the master branch, or the last release, it gives me an error. When I use @Yannic's fork, it uses the correct renaming according to the map. I know it sounds weird, do you know why it happens? Maybe the SubstitutionMap is initialised more than once, sometimes using a correct class, and sometimes not?

elipoultorak commented 5 years ago

Update: it also works with json maps.

Yannic commented 5 years ago

AFAIK, the crash above happens because the map isn't initialized at all. You're right that my fix works, but the map is only initialized right before the crash would happen and not where the initialization is supposed to happen. Unfortunately, that's all I remember right now. If I remember to do so, I'll have a look (next week or so) if I can find some notes from two years ago.

elipoultorak commented 5 years ago

That makes more sense, but it doesn't explain why the code works. :-)

my-code-doesnt-work-i-have-noidea-why-my-code-33594653

sgammon commented 5 years ago

@elipoultorak @Yannic sick, ha ha another one bites the dust, good work @Yannic

sgammon commented 5 years ago

i have a fork of closure-stylesheets that gathers most, if not all, of these fixes and features introduced via PRs. i'll merge this in today, feel free to ping me if our fork can be helpful to you in the meantime before this gets merged to master upstream.

elipoultorak commented 5 years ago

Looks good, but is nobody at Google merging anything to this repo?

sgammon commented 5 years ago

@elipoultorak welcome to the gap between regular open source, and google internal :) lol