On Tue, 3 Feb 2026 10:07:59 GMT, Jan Lahoda <[email protected]> wrote:

>> Another thing about these overloads I just realized is that before and after 
>> this change there is an overload that takes three boolean parameters, but 
>> the meaning of them has changed: `keepDocComments, keepEndPos, keepLineMap` 
>> vs `keepDocComments, keepLineMap, parseModuleInfo`.
>> 
>> I'd forgotten to update some call sites because they still compiled. I have 
>> fixed them now, and it didn't matter in those cases because they were 
>> passing `false` for all of the options, but it does show these overloads 
>> could be bug-prone.
>> 
>> I'd be happy to try to improve this as a follow up if you think that'd be 
>> worthwhile. Having a single `newParser` overload could be safer because it 
>> forces callers to update if the signature changes again. Another possibility 
>> might be something like 
>> `parserFactory.newParser(input).keepDocComments(true).keepLineMap(true).parse()`.
>
> For me - I think there was a time where the idea was that some of the 
> internal APIs (like to parser factories) would be evolved somewhat 
> "compatibly". I don't think that's necessary anymore (and compatible 
> evolution is not being done anyway), esp. for things like parser factories. 
> So, for me: we can clear the factories and only keep one, that's OK and will 
> improve maintainability.

That sounds good to me, I went ahead and updated this to just have the one 
overload.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28610#discussion_r2758329410

Reply via email to