antlr4ts icon indicating copy to clipboard operation
antlr4ts copied to clipboard

Can we simplify TokenStreamRewriter's overloads?

Open BurtHarris opened this issue 9 years ago • 1 comments

The set of overloads in TokenStreamRewriter.tssoon` is seems over-complicated. I'm wondering if a little refactoring is in order, at least for the TypeScript port.

  1. The optional programName argument at the beginning of insertAfter, insertBefore, replace and delete methods seems unnecessary. Instead, clients using this API could create separate instances of TokenStreamRewriter, each one serving as it's own "program".

  2. The token index are overloaded with both number and Token type seem like they could be replaced with number | Token types, further reducing the number of overloads.

  3. Where the API includes a text argument, is there any reason this shouldn't be typed string

Thus I'd suggest the side-effecting methods be reduced to:

insertAfter(index: number|Token, text: string): void;
insertBefore(index: number|Token, text: string): void;
replace(index: number|Token, text: string): void ;
replace(from: number|Token, to: number|Token, text: string): void ;
delete(from: number|Token, to?: number|Token): void;

This seems like a pretty complete set of operations, but the class as currently defined seems to support sub-classing it, which exposes substantially more surface area (e.g. the nested operation classes). Was a useful application for sub-classing TokenStreamRewriter discovered in Java?

BurtHarris avatar Oct 23 '16 21:10 BurtHarris

Years later, answering my own question. Yes, I'm convinced the overloads are wrong for TypeScript when optional parameters can be used. and TokenStreamRewriter is not the only case of this. I believe this calls for a review of the whole codebase.

BurtHarris avatar Apr 15 '20 19:04 BurtHarris