Support ANTLR 3.5

[marcus-nl]

In ANTLR 3.5 the field Token.EOF_TOKEN has been removed, which causes a runtime error because it's used here:

https://github.com/SomMeri/less4j/blob/master/src/main/antlr3/com/github/sommeri/less4j/core/parser/Less.g#L138

In version 3.5 obtaining this token is done through Lexer#getEOFToken().

[SomMeri]

Hi, how important is that for you? Is it needed or rather nice to have?

[marcus-nl]

It's an annoyance because we use ANTLR 3.5, forcing us to downgrade it to 3.4 in our frontend. So not really a big deal, but nice to have.

[alexo]

@marcusk24 you could use JarJar maven plugin to solve this problem.

[SomMeri]

@marcusk24 I'm working on extend #151 now and would like to finish it before I start something new. Of course, I would accept pull request if anyone would have time to do it :).

[jochenberger]

I've updated ANTLR to 3.5.1 in my repo: https://github.com/jochenberger/less4j/tree/update-antlr-to-3.5
It causes lots of test failures though, most of them related to appenders and white-space. I'm not too familiar with ANTLR, so I'd appreciate any hints you might be able to give me.

[SomMeri]

@jochenberger Unfortunately, I'm short on time right now and will not be able to get to it this week.

Anyway, appenders are done in a non-standard way:

fragment APPENDER_FRAGMENT: '&';
fragment MEANINGFULL_WHITESPACE: ;

APPENDER: ws1=WS_FRAGMENT? app=APPENDER_FRAGMENT ws2=WS_FRAGMENT? {
  emitAs($ws1, MEANINGFULL_WHITESPACE); 
  emitAs($app, APPENDER);
  emitAs($ws2, MEANINGFULL_WHITESPACE);
};

where three calls of method emitAs change tokens type. Eg. emitAs($ws1, MEANINGFULL_WHITESPACE); changes the type of $ws1 into MEANINGFULL_WHITESPACE type.

I guess that this stopped to work as expected after upgrade.

I did it this way, spaces are normally insignificant and I throw them away. However, they are significant in some places and this is one of them. The above is a "hack" to make them matter here.

[jochenberger]

Yes, the failures are probably related. For example, interpolation-selector.less in EscapingAndInterpolationTest fails.
The less fragment is

//nested with appender
.red {
  #@{theme}.@{theme}&.black {
    color:black;
  }
}

the expected result is

#blood.blood.red.black {
  color: black;
}

but it produces

#blood.blood.red .black {
  color: black;
}

[SomMeri]

The emitAs method has following check:

      public void emitAs(Token token, int type) {
            if (token==null) // this probably stopped to work
              return ;
            token.setType(type);
            emit(token);
      }

I guess that the token is not null if there is no whitespace anymore for some reason. The method is defined in less.g file. Antlr copies it into generated LessLexer class, so you can put breakpoint there.

[jochenberger]

I suspected that too, but token is null in the relevant cases.

[UnquietCode]

I just hit the same issue and started working on this. Is it worth doing an upgrade to 3.5 or do you think it's more worthwhile to jump to the antlr 4 line?

[SomMeri]

@UnquietCode Is there a reason not to jump to antlr 4? I did not researched it. If there is no reason, then I think it should be worth it.

[UnquietCode]

Mainly it just looks quite a bit different. Different file extensions, different generated classes, "ground up rewrites" of their core classes, etc. Is it worth it? Definitely, especially as Antlr 3 won't be around forever. However my guess is this is probably a task requiring intimate knowledge of less4j which you ( @SomMeri ) would have to tackle. I'll make a fork and start exploring though in the meantime.

[UnquietCode]

Ok yeah the grammar in antrl4 seems to have changed quite a bit, and the tree concept has changed as well. Here is some initial work which updates the dependencies, plugin goals, basic package renaming, etc. There are still a lot of compile errors due to the missing classes.

https://github.com/UnquietCode/less4j/commits/master

[SomMeri]

@UnquietCode If you found a good write up about changes would you post it here? I have seen a presentation about it and it seemed like antlr 4 could allow me to remove some hacks/fragile places from less4j grammar. Plus, they promised that 4.2 will be faster.

I would accept both move to 3.5 and move to 4.2. However, working on 3.5 might be waste of time if the thing will be moved on 4.2 later on.

[UnquietCode]

I had looked for a guide originally and could not find one, so I raised a question on the antlr 4 issue tracker.

antlr/antlr4#464

[SomMeri]

Thank you a lot. I subscribed to that antlr issue.

[SomMeri]

tl;dr: As attractive as this task seems to be, I do not see myself doing it this winter. I would be thankful for pull request if anyone else does it.

Based on the antlr4 answer, move to antlr 4 is possible but probably a lot of work. I seems like interesting challenge for someone with a lot of time.

Less4j uses ^() and made up tokens heavily, so the most work will be caused by: abstract syntax tree (AST) construction is no longer an option. We will have to rewrite many rules in the grammar and each will require corresponding modification in ASTBuilderSwitch.

Although most changes will probably be trivial and straightforward, they will have to be done all at the same time as I do not see a way how to do them one by one. So it looks like the case of do a lot of coding and then debug for a long time.

Another thing that will need to be rewritten is comments handing - how it works now is described here.

On the plus side, adding new less features into antlr4 grammar will probably be easier then adding them into current one.

[SomMeri]

Another potential problem: predicates. Less4j uses them a lot and if their handling changed, there might be a tricky error.

Edited to add if.

[felixscheffer]

I am running into the same problem as marcusk24. It would be much appreciated if you could upgrade to either 3.5 or 4.x.

[SomMeri]

@fscheffer Is it possible for you to give me less sheet example that causes the problem? Even if we would start move to 4.x tomorrow, which is physically impossible, it would take a lot of time to finish it.

If I would have example sheet, there might try to find faster solution.

[felixscheffer]

There are no problems regarding less sheets.

We have two dependencies in our project, StringTemplate4 and Tapestry5, that also depend on antlr-runtime. Both projects use antlr-runtime 3.5 at the moment, so it would be great if you could upgrade less4j to 3.5 too.

Right now we have to downgrade to antlr-runtime manually to 3.4 to make less4j happy. Luckily both StringTemplate4 and Tapestry5 seem to work fine with antlr-runtime 3.4 but I am very concerned that this wont be the case in the (near) future.

(Sorry for the late response)

[SomMeri]

That sounds like a strong argument against move to antlr4 yet. If it is going to conflict (I'm not sure) with other antlr3 libraries, then we can not upgrade to it. I guess I will be able to bump up the priority of this after I finish missing functions. That seems like the last major missing language feature.

[jochenberger]

It seems all ANTLR 4 classes are inside the org.antlr.v4 package, so there shouldn't be any compatibility issues with antlr3.
http://www.antlr.org/api/Java/org/antlr/v4/runtime/package-tree.html

[alexo]

@SomMeri I would suggest repackage antlr inside less4j (using something like JarJar Maven Plugin: http://sonatype.github.io/jarjar-maven-plugin/). As result, any project using less4j won't have any compatibility issues.

[felixscheffer]

I think alexo's solution is propably the easiest one at the moment. I would not upgrade to 3.5 or 4.x until there's a real benefit.

[jochenberger]

antlr/antlr3#151 might cause many people to switch to 3.5 at least.

[SomMeri]

I pulled the upgrade to antlr 3.5.2 and closing this issue. The upgrade to antlr4 is much bigger task, so I opened separate issue #223 for it.