Продолжение рефакторинга под актуальную версию java и фиксы для null check#38
Продолжение рефакторинга под актуальную версию java и фиксы для null check#38theshadowco merged 6 commits intodevelopfrom
Conversation
- продолжение корректировок под null safe
WalkthroughМассовая миграция аннотаций nullness на JSpecify: добавлены Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/main/java/org/antlr/v4/tool/Rule.java (1)
288-304:⚠️ Potential issue | 🔴 CriticalПотенциальный NPE:
g.getRule(...)может вернутьnull.На строке 292 вызывается
g.getRule(anyLabelDef.element.getText()).resolveRetvalOrProperty(y), ноGrammar.getRule(String)теперь аннотирован как@Nullable(см.Grammar.java, строка 526). Это приведёт кNullPointerException, если правило не найдено.🐛 Предлагаемое исправление
if (anyLabelDef != null) { if (anyLabelDef.type == LabelType.RULE_LABEL) { - return g.getRule(anyLabelDef.element.getText()).resolveRetvalOrProperty(y); + Rule rule = g.getRule(anyLabelDef.element.getText()); + if (rule == null) { + return null; + } + return rule.resolveRetvalOrProperty(y); } else {src/main/java/org/antlr/v4/runtime/IncrementalParser.java (1)
65-74:⚠️ Potential issue | 🟠 MajorВозможна разбалансировка стека
minMaxStackприLT(1) == null.Если
_input.LT(1)вернётnull, методpushCurrentTokenToMinMaxзавершится без вызоваincrementalTokenStream.pushMinMax(...). ОднакоenterEveryRule(строка 163) всегда вызывает этот метод, аexitEveryRule(строка 175) всегда вызываетpopAndHandleMinMax. Это приведёт кIndexOutOfBoundsExceptionвpopMinMax()из-за разбалансировки стека.На практике
LT(1)дляBufferedTokenStreamне должен возвращатьnullприk=1, но если вы обрабатываетеnullздесь, стоит обеспечить согласованность push/pop:🐛 Вариант исправления
var token = _input.LT(1); - if (token != null) { - incrementalTokenStream.pushMinMax(token.getTokenIndex(), token.getTokenIndex()); - } + incrementalTokenStream.pushMinMax( + token != null ? token.getTokenIndex() : Integer.MAX_VALUE, + token != null ? token.getTokenIndex() : Integer.MIN_VALUE + );src/main/java/org/antlr/v4/runtime/BufferedTokenStream.java (1)
196-199:⚠️ Potential issue | 🟠 MajorПотенциальный NPE в
LA(int i): вызовLT(i).getType()без null-проверки, хотяLTаннотирован@Nullable.
LT(int k)(строка 211) помечен@Nullable, ноLAвызывает.getType()на результате без проверки. Это может привести кNullPointerException, еслиLTвернётnull(например, приk == 0). Для справки: вAbstractBaseTest.IntTokenStream.LAиспользуетсяObjects.requireNonNull(LT(i)).getType().🛡️ Предлагаемое исправление
`@Override` public int LA(int i) { - return LT(i).getType(); + return Objects.requireNonNull(LT(i)).getType(); }src/test/java/org/antlr/v4/test/tool/AbstractBaseTest.java (1)
999-1002:⚠️ Potential issue | 🟠 Major
getTokenSource()возвращаетnullв классе с@NullMarked.Класс
IntTokenStreamаннотирован@NullMarked, что подразумевает non-null возвращаемые значения по умолчанию. МетодgetTokenSource()возвращаетnull, нарушая этот контракт. Необходимо добавить@Nullableна возвращаемый тип или бросить исключение.🐛 Предлагаемое исправление (вариант 1: `@Nullable`)
`@Override` + `@Nullable` public TokenSource getTokenSource() { return null; }🐛 Предлагаемое исправление (вариант 2: исключение)
`@Override` public TokenSource getTokenSource() { - return null; + throw new UnsupportedOperationException("IntTokenStream does not have a token source"); }src/main/java/org/antlr/v4/runtime/atn/ParserATNSimulator.java (1)
874-874:⚠️ Potential issue | 🟡 MinorПотенциальный NPE при
previous.s0 == null.Строка 874 обращается к
previous.s0.configsбез проверки на null. Еслиprevious.s0равенnull(условие на строке 819 ложно), произойдётNullPointerException. Это предшествующая проблема, но в контексте миграции на null-safety стоит рассмотреть добавление защиты.src/main/java/org/antlr/v4/runtime/UnbufferedTokenStream.java (1)
288-309:⚠️ Potential issue | 🟡 MinorПредшествующая проблема:
getText(Interval)используетtokens.lengthвместоn.Строка 290:
bufferStopIndex = bufferStartIndex + tokens.length - 1— расчёт границы основан на ёмкости массива, а не на количестве фактически заполненных элементов (n). При обращении кtokens[i]дляi >= nбудет полученnull, что под@NullMarkedприведёт к нарушению контракта. Это предшествующая проблема, не введённая данным PR, но может стоить отдельного issue.src/main/java/org/antlr/v4/automata/ParserATNFactory.java (1)
346-362:⚠️ Potential issue | 🟠 MajorNPE-риск:
currentRuleбез null-проверки вsempred()иaction().В строке 356 (
currentRule.index) и строке 372 (currentRule.index) полеcurrentRuleразыменовывается без проверки наnull, хотя оно аннотировано как@Nullable. Вероятнее всего, при нормальном потоке выполненияcurrentRuleвсегда установлено к моменту вызова этих методов, но контракт@Nullableэто не гарантирует.Рекомендую либо добавить guard-условие с выбросом
IllegalStateException, либо задокументировать предусловие.Пример защитной проверки
`@Override` public Handle sempred(PredAST pred) { var left = newState(pred); var right = newState(pred); + if (currentRule == null) { + throw new IllegalStateException("currentRule must be set before calling sempred"); + } AbstractPredicateTransition p;
🤖 Fix all issues with AI agents
In `@src/main/java/org/antlr/v4/automata/ATNFactory.java`:
- Line 179: The interface method signature Handle lexerCallCommand(GrammarAST
ID, GrammarAST arg) must mark the second parameter as nullable to match call
sites that pass null; update the signature to use `@Nullable` on the arg parameter
(e.g., Handle lexerCallCommand(GrammarAST ID, `@Nullable` GrammarAST arg)) so it
aligns with lexerCommand delegating with null and the `@NullMarked` contract used
in LexerATNFactory.java and related implementations.
In `@src/main/java/org/antlr/v4/automata/ParserATNFactory.java`:
- Around line 731-736: The newState(`@Nullable` GrammarAST) method dereferences
currentRule.index without a null check causing an NPE; update newState(`@Nullable`
GrammarAST) (and the BasicState creation path) to mirror the overload
newState(Class<T>, `@Nullable` GrammarAST) by testing if currentRule is null and
only call n.setRuleIndex(currentRule.index) when non-null, otherwise set the
state rule index to the ATN invalid/default value (e.g., ATN.INVALID_RULE_INDEX
or -1) before adding to atn.
- Around line 145-151: Replace the repeated calls to pair.getItem1() with the
already null-checked local variable item1 in the ParserATNFactory.java block:
use item1.ast and item1.name when calling g.tool.errMgr.grammarError (and the
cast to (GrammarAST) item1.ast.getChild(0)) instead of calling pair.getItem1()
again, so the code consistently uses the local item1 variable.
In `@src/main/java/org/antlr/v4/runtime/atn/ParserATNSimulator.java`:
- Line 226: Add the JSpecify `@NullMarked` annotation to the ParserATNSimulator
class: import org.jspecify.nullness.NullMarked and annotate the class
declaration for ParserATNSimulator with `@NullMarked` (placed immediately above
the existing "public class ParserATNSimulator extends ATNSimulator" line) so it
matches the nullness annotations used by ATNSimulator and other classes in the
runtime.atn package.
In `@src/main/java/org/antlr/v4/runtime/IncrementalParser.java`:
- Around line 104-105: The call in IncrementalParser that uses
Objects.requireNonNull(_input.LT(1)).getTokenIndex() is inconsistent with
pushCurrentTokenToMinMax which quietly handles a null LT(1); make the handling
uniform: either remove the requireNonNull and defend against a null LT(1) (e.g.,
handle the null case and pass a sentinel or skip calling getTokenIndex) or
update pushCurrentTokenToMinMax to throw/propagate when LT(1) is null. Locate
the occurrences by searching for _input.LT(1), Objects.requireNonNull and the
method pushCurrentTokenToMinMax, then apply one consistent null strategy across
both call sites so LT(1) is treated identically.
In `@src/main/java/org/antlr/v4/runtime/Lexer.java`:
- Around line 219-225: setInputStream currently only calls reset() when type !=
Token.INVALID_TYPE, which can leave other lexer state dirty (hitEOF, modeStack,
channel) if type was set to INVALID_TYPE; change setInputStream to perform a
full reset unconditionally (i.e., always call reset()), or at minimum expand the
condition to reset when hitEOF is true or !modeStack.isEmpty() or channel != 0
or type != Token.INVALID_TYPE; locate the method setInputStream and the reset()
invocation and update the conditional to ensure reset() runs in those
dirty-state cases (you may also adjust/mention setType if Lombok-generated
setters can set type to INVALID_TYPE).
In `@src/main/java/org/antlr/v4/runtime/misc/Tuple.java`:
- Around line 29-31: The factory method create for Tuple3 is missing `@Nullable`
on T2 and T3; update the signature of public <T1, T2, T3> Tuple3<@Nullable T1,
T2, T3> create(`@Nullable` T1 item1, T2 item2, T3 item3) to annotate both type
parameters and parameters as nullable (i.e., <@Nullable T1, `@Nullable` T2,
`@Nullable` T3> and Tuple3<@Nullable T1, `@Nullable` T2, `@Nullable` T3> and `@Nullable`
on item2 and item3) so it matches the Tuple3 class declaration, constructor, and
fields.
In `@src/main/java/org/antlr/v4/runtime/Parser.java`:
- Around line 185-189: The code assigns null to the field _ctx (type
ParserRuleContext) which is declared non-null under `@NullMarked`; fix by either
marking the field as `@Nullable` or avoiding null assignment—preferably
create/assign a safe empty ParserRuleContext placeholder (e.g., new
ParserRuleContext()) where the reset occurs (in method containing the lines with
created/_errHandler.reset(this)/_ctx = null) and update any callers if needed,
or alternatively update the field declaration to protected `@Nullable`
ParserRuleContext _ctx to reflect nullable semantics; pick one approach and
apply consistently (adjust imports/annotations or create the stub instance) to
restore JSpecify contract correctness.
In `@src/main/java/org/antlr/v4/runtime/Recognizer.java`:
- Around line 144-146: The getParseInfo() method throws the wrong exception type
and has a typo in the message; replace the thrown IllegalCallerException("Not
implemended") with UnsupportedOperationException("Not implemented") so the
semantics match getSerializedATN() and the message is corrected; update the
throw expression in the getParseInfo method accordingly.
In `@src/main/java/org/antlr/v4/runtime/RuleContext.java`:
- Around line 73-74: The class invariant that a root has parent==null and
invokingState==-1 is violated by tests, so update RuleContext to enforce and
clarify behavior: (1) Add constructor validation in the
RuleContext/ParserRuleContext constructor(s) to throw IllegalArgumentException
when parent==null but invokingState!=-1 (or normalize by setting invokingState
to -1 if you prefer consistency); (2) Change isEmpty() to reflect the true
empty/root check by returning parent==null && invokingState==-1 instead of only
testing invokingState; and (3) update the RuleContext Javadoc for the class and
isEmpty() to describe this invariant and the constructor behavior so
callers/tests must either supply a non-null parent for non-root states or use
invokingState -1 for roots. Reference symbols: RuleContext, isEmpty(),
ParserRuleContext (constructors).
In `@src/main/java/org/antlr/v4/tool/ast/GrammarASTWithOptions.java`:
- Around line 73-79: getOptionInt currently calls Integer.parseInt(option)
(after getOptionString) and can throw NumberFormatException for malformed
numbers; wrap the parse call in a try/catch catching NumberFormatException and
return a safe default (e.g., 0) or rethrow a clearer exception, and optionally
log the bad option key/value for debugging; update getOptionInt and reference
getOptionString and Integer.parseInt when making the change, or if you prefer
fail-fast, add javadoc to getOptionInt documenting that it throws
NumberFormatException for invalid numeric option values.
In `@src/main/resources/org/antlr/v4/tool/templates/codegen/Java/Java.stg`:
- Line 991: В шаблоне `AltLabelStructDecl` заменить генерацию полей атрибутов с
`<attrs:{a | public <a>;}; separator="\n">` на вариант с аннотацией nullable
аналогично `StructDecl` (т.е. использовать `public `@Nullable` <a>;`), чтобы
унифицировать контракт nullable для всех генерируемых контекстных классов;
проверьте шаблон `AltLabelStructDecl` (около строки 1017) и внесите ту же
правку, что и в `StructDecl`.
In `@src/test/java/org/antlr/v4/test/tool/PerformanceTest.java`:
- Around line 1295-1298: The parser setup mistakenly assigns a lexer ATN array
to the parser ATN: when computing ATN in the parser initialization block
(variables/methods involved: parser, previousParser, atn, REUSE_PARSER_DFA,
FILE_GRANULARITY, sharedLexerATNs, sharedParserATNs), replace the incorrect use
of sharedLexerATNs[thread] with sharedParserATNs[thread] so the parser receives
a parser ATN when REUSE_PARSER_DFA is false (and the other conditional holds);
keep the existing conditional structure but swap to sharedParserATNs to match
the intended behavior.
In `@src/test/java/org/antlr/v4/test/tool/SemPredEvalParserTest.java`:
- Around line 419-430: Remove the trailing backslash line-continuation
characters from the grammar text block in SemPredEvalParserTest.java (e.g. the
"a[int i]\" line and similar occurrences inside the grammar T text block where
rules like "a[int i]\" appear), so each grammar rule is on its own physical
line; edit the string literal used in the test (the text block assigned to the
grammar for the test case) to delete the '\' at ends of lines (lines noted in
the review: occurrences near the "a[int i]\" rule and other matches) to restore
normal newlines and improve readability and consistency.
🧹 Nitpick comments (15)
src/test/java/org/antlr/v4/test/tool/TokenStreamRewriterTest.java (1)
614-623: Несогласованный стиль проверки исключений вtestReplaceThenReplaceLowerIndexedSuperset.В остальных тестах файла (строки 329, 504, 591, 804) используется
assertThatThrownBy(tokens::getText), а здесь — ручнойtry-catchс последующимиassertThat. Для единообразия стоит привести к тому же стилю.♻️ Предлагаемый рефакторинг
- Exception exc = null; - try { - tokens.getText(); - } catch (IllegalArgumentException iae) { - exc = iae; - } String expecting = "replace op boundaries of <ReplaceOp@[`@1`,1:1='b',<2>,1:1]..[`@3`,3:3='c',<3>,1:3]:\"foo\"> " + "overlap with previous <ReplaceOp@[`@2`,2:2='c',<3>,1:2]..[`@4`,4:4='c',<3>,1:4]:\"xyz\">"; - assertThat(exc).isNotNull(); - assertThat(exc.getMessage()).isEqualTo(expecting); + + assertThatThrownBy(tokens::getText) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage(expecting);src/test/java/org/antlr/v4/test/tool/PerformanceTest.java (1)
840-849: ВозвратnullизCallableпри исключении — потенциальный NPE.Если выброшено исключение в лямбде, возвращается
null. Далее на строке 860 результатfuture.get()используется без null-проверки (строка 862:fileResult.parserTotalTransitions), что приведёт кNullPointerException. Это поведение было и до рефакторинга, но стоит рассмотреть добавление null-проверки, особенно учитывая фокус PR на null-safety.♻️ Предлагаемое улучшение
try { FileParseResult fileResult = future.get(); + if (fileResult == null) { + continue; + } if (COMPUTE_TRANSITION_STATS) {src/main/java/org/antlr/v4/runtime/RuleContext.java (1)
39-40: Форматирование Javadoc примера можно улучшить.Разбиение примера кода на две строки в середине выражения снижает читаемость. Рассмотрите возможность форматирования примера более структурированно, например, разделяя по логическим операциям (вызов метода, выход из правила, возврат значения).
♻️ Предлагаемое улучшение форматирования
- * create new node enterRule(_localctx, 0, RULE_s); -- push it ... exitRule(); -- pop back to - * _localctx return _localctx; } + * create new node + * enterRule(_localctx, 0, RULE_s); // push it + * ... + * exitRule(); // pop back to _localctx + * return _localctx; + * }src/test/java/org/antlr/v4/test/tool/SetsTest.java (2)
301-312: Непреобразованный тест — незначительная неконсистентность.
testNotCharSetWithRuleRef3остался с конкатенацией строк, тогда как все остальные тесты в файле переведены на text blocks. Вероятно, это намеренно из-за inline-комментария в строке 306, но стоит унифицировать для единообразия.
273-278: Грамматики в массивах ошибок — отличающийся стиль отступов.В
testNotCharSetWithRuleRefиtestNotCharSetWithStringtext blocks начинаются без отступов (колонка 0), тогда как все остальные грамматики в файле используют отступ в 8 пробелов. Семантически это корректно, но визуально отличается. Вероятно, сделано для точного совпадения с текстом ожидаемых ошибок — если так, то всё ок.Also applies to: 289-294
src/test/java/org/antlr/v4/test/tool/SemPredEvalParserTest.java (1)
96-121: Частичная конвертация: грамматика осталась конкатенацией, а expected output — text block.В
testValidateInDFAграмматика (строки 98–108) осталась конкатенацией строк, в то время как expectedstderrDuringParse(строки 116–119) переведён на text block. Аналогичная ситуация в нескольких других тестах (testSimple,testOrder,test2UnpredicatedAlts, и т.д.). Незначительная непоследовательность — можно унифицировать в рамках этого PR или позже.src/main/java/org/antlr/v4/runtime/misc/Tuple3.java (1)
19-30:@Valueгенерируетequals/hashCode/toString, которые перекрываются явными реализациями ниже.
@Valueнеявно добавляет@EqualsAndHashCodeи@ToString, но здесь они переопределены вручную (строки 32–58). Это работает корректно, однако Lombok всё равно генерирует код, который сразу перекрывается. Для согласованности с намерением можно рассмотреть замену@Valueна@Getter+ ручной конструктор (как уже есть), либо подавить генерацию:♻️ Предлагаемое изменение
-@Value +@Value +@EqualsAndHashCode(doNotUseGetters = true) +@ToStringИли проще — заменить
@Valueна набор аннотаций без лишней генерации:-@Value +@GetterЭто аналогично
Pair.java, где тоже используется@Valueс явнымиequals/hashCode/toString— стоит привести оба класса к единому подходу.src/main/java/org/antlr/v4/runtime/IncrementalParser.java (1)
60-62: Можно упроститьaddAndGet(1)→incrementAndGet().♻️ Предлагаемое изменение
protected static int incrementGlobalParserEpoch() { - return IncrementalParser._PARSER_EPOCH.addAndGet(1); + return IncrementalParser._PARSER_EPOCH.incrementAndGet(); }src/main/java/org/antlr/v4/runtime/CommonTokenStream.java (1)
12-13: Смешение старого@NotNullи нового@Nullableиз jspecify.В файле одновременно используются
org.antlr.v4.runtime.misc.NotNull(строки 55, 69) иorg.jspecify.annotations.Nullable. Желательно в рамках миграции заменить и старые@NotNullна jspecify-аннотации для единообразия.src/main/java/org/antlr/v4/runtime/DefaultErrorStrategy.java (1)
540-548: Рекомендуется добавить сообщение вObjects.requireNonNull.
Objects.requireNonNull(current.getTokenSource())без сообщения даст NPE без указания причины. Добавление описательного сообщения упростит отладку.♻️ Предлагаемое исправление
- factory.create(Tuple.create(tokenSource, Objects.requireNonNull(current.getTokenSource()).getInputStream()), + factory.create(Tuple.create(tokenSource, Objects.requireNonNull(current.getTokenSource(), "current token has no token source").getInputStream()),src/main/java/org/antlr/v4/runtime/atn/Transition.java (1)
76-78: Удалена runtime проверка на null в конструктореTransition.Ранее конструктор содержал явную проверку
targetнаnull. С@NullMarkedконтракт обеспечивается на уровне статического анализа, но runtime-защита потеряна. Учитывая, что полеtarget—publicи мутабельное, в runtime никто не мешает присвоитьnull. Если это осознанное решение — рекомендую добавитьObjects.requireNonNull(target)для согласованности с подходом, применённым вUnbufferedTokenStream.LA()(строка 115), где используетсяObjects.requireNonNull.🛡️ Предлагаемое изменение
protected Transition(ATNState target) { + this.target = Objects.requireNonNull(target); - this.target = target; }src/main/java/org/antlr/v4/runtime/Parser.java (3)
169-178: Полеcreatedи порядок инициализации — корректно, но документацию стоит улучшить.Паттерн с
createdдля обхода проблемы вызоваreset()из конструктора (когдаinterpreterещё не инициализирован) работает правильно. Однако:
- Комментарий к полю (строки 170-171) написан на русском — для open-source проекта с международной аудиторией стоит использовать английский.
- Поле
created— это фактически флаг «полностью инициализирован», и его имя не полностью отражает назначение. Имя вродеinitializedилиfullyConstructedбыло бы яснее.Also applies to: 184-197
511-517:IllegalCallerExceptionсемантически не подходит.
IllegalCallerExceptionпредназначен для stack-walking API (Java 9+) — когда вызывающий модуль не имеет прав на операцию. Здесь же ошибка в том, что парсер не инициализирован — это состояние объекта, а не проблема вызывающего.IllegalStateExceptionбыл бы корректнее.Предлагаемое исправление
- throw new IllegalCallerException("Parser not initialized"); + throw new IllegalStateException("Parser not initialized");
911-918:ClassCastExceptionвводит в заблуждение.Исключение
ClassCastExceptionподразумевает неудачное приведение типов, но здесь никакого приведения нет — просто интерпретатор не являетсяProfilingATNSimulator. Более подходящим было быIllegalStateExceptionилиUnsupportedOperationException.Предлагаемое исправление
- throw new ClassCastException("Incorrect getInterpreter()"); + throw new UnsupportedOperationException("Parser interpreter is not a ProfilingATNSimulator. Call setProfile(true) first.");src/main/java/org/antlr/v4/automata/LexerATNFactory.java (1)
212-213: Несоответствие nullability параметраargмежду интерфейсом и реализацией.В
ATNFactory(строка 179) методlexerCallCommand(GrammarAST ID, GrammarAST arg)объявляетargкак non-null (под@NullMarked). Здесь же вLexerATNFactoryпараметр аннотирован@Nullable. Формально это допустимо с точки зрения LSP (реализация принимает более широкий набор значений), однако это может вводить в заблуждение и вызывать предупреждения в инструментах проверки null-safety.Стоит привести интерфейс в соответствие — аннотировать
argкак@NullableвATNFactory.lexerCallCommandтоже, тем более чтоlexerCommandделегирует вызов сnull.
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2418 🏁 Script executed: Length of output: 779 🏁 Script executed: Length of output: 778
ПроблемаВ файле - uses: actions/checkout@v6
with:
fetch-depth: 0 # <-- Это уже клонирует полный репозиторий
- run: |
git fetch --prune --unshallow # <-- Эта команда пытается "разгрузить" shallow репозиторийОшибка возникает потому, что:
РешениеНужно удалить избыточный шаг с - uses: actions/checkout@v6
with:
fetch-depth: 0
# Удалить следующие строки:
# - run: |
# git fetch --prune --unshallow
- name: Set up JDK 17
uses: actions/setup-java@v5
# ... остальное без измененийПараметр |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/antlr/v4/automata/ParserATNFactory.java (1)
485-490:⚠️ Potential issue | 🟠 MajorПроверка на
nullбез раннего выхода — потенциальный NPE.Строки 487–489 сообщают об ошибке, если
firstилиlastравенnull, но выполнение продолжается до строки 490, где происходит обращение кfirst.left/last.right, что приведёт к NPE.Предлагаемое исправление
var first = els.get(0); var last = els.get(n - 1); if (first == null || last == null) { - g.tool.errMgr.toolError(ErrorType.INTERNAL_ERROR, "element list has first|last == null"); + throw new IllegalStateException("element list has first|last == null"); } return new Handle(first.left, last.right);
🤖 Fix all issues with AI agents
In `@src/main/java/org/antlr/v4/automata/ParserATNFactory.java`:
- Around line 369-376: The code dereferences nullable field currentRule in
action(ActionAST) (currentRule.index) and similarly in sempred(...); add a
null-check for currentRule at the start of both methods (action and sempred) and
fail fast when it's null—throw an IllegalStateException with a clear message
(e.g., "currentRule is null while creating ActionTransition" / "currentRule is
null in sempred") or otherwise obtain a valid rule index before constructing
ActionTransition/SemanticPredicate; reference the currentRule field and the
ActionTransition/SemanticPredicate construction points to locate where to add
the guard.
In `@src/test/java/org/antlr/v4/test/tool/PerformanceTest.java`:
- Around line 840-849: В текущем lambda для executorService.submit (создающий
Future<FileParseResult> futureChecksum) при исключении catch печатает стек и
возвращает null, из-за чего future.get() может вернуть null и вызвать NPE при
обращении к fileResult.parserTotalTransitions; исправьте catch в этой лямбде
(где вызывается factory.parseFile(...)) так, чтобы не возвращать null — либо
пробросьте исключение (обернув Throwable в RuntimeException и rethrow), либо
верните корректный FileParseResult с полем ошибки; то есть измените поведение в
блоке catch для лямбды, связанной с executorService.submit / futureChecksum,
чтобы исключение корректно поднималось через Future вместо возврата null.
🧹 Nitpick comments (1)
src/main/java/org/antlr/v4/automata/ParserATNFactory.java (1)
278-281: ЗаменаUnsupportedOperationExceptionнаIllegalCallerException— непоследовательно.
charSetLiteralбросаетIllegalCallerException(строка 280), тогда как остальные методы, запрещённые для парсера (lexerAltCommands,lexerCallCommand,lexerCommand,action(String)), бросаютUnsupportedOperationException. Стоит использовать один тип исключения для единообразия.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/antlr/v4/automata/ParserATNFactory.java (1)
485-491:⚠️ Potential issue | 🔴 CriticalNPE: после проверки
first/lastнаnullвыполнение продолжается с разыменованием.Строка 487 проверяет
first == null || last == nullи логирует ошибку, но не прерывает выполнение. На строке 490first.leftиlast.rightразыменовываются безусловно, что приведёт кNullPointerException.Предлагаемое исправление
var first = els.get(0); var last = els.get(n - 1); if (first == null || last == null) { - g.tool.errMgr.toolError(ErrorType.INTERNAL_ERROR, "element list has first|last == null"); + throw new IllegalStateException("element list has first|last == null"); } return new Handle(first.left, last.right);src/main/java/org/antlr/v4/runtime/Parser.java (1)
502-505:⚠️ Potential issue | 🟠 Major
reset()применяется к старому потоку перед установкой нового — новый поток никогда не сбрасывается на позицию 0.Когда вызывается
setTokenStream(TokenStream input), методreset()вызывает_input.seek(0)на текущем (старом) потоке, а затем присваивает новый поток полю_input. Если новый поток не находится в начальной позиции, парсер начнёт парсинг с неправильной позиции. Документация метода обещает "reset the parser", что логически должно включать сброс потока на позицию 0.Предлагаемое исправление
public void setTokenStream(TokenStream input) { + this._input = input; reset(); - this._input = input; }
🤖 Fix all issues with AI agents
In `@src/main/java/org/antlr/v4/runtime/Parser.java`:
- Around line 911-918: Метод getParseInfo использует instanceof
ProfilingATNSimulator но при несоответствии кидает ClassCastException —
семантически неверно и ломает обратную совместимость (раньше мог возвращать
null). Исправьте реализацию в getParseInfo: если getInterpreter() не является
ProfilingATNSimulator, восстановите поведение назад и верните null (чтобы не
ломать вызывающий код); если вы действительно хотите выбрасывать исключение,
замените ClassCastException на более подходящий тип, например
IllegalStateException или UnsupportedOperationException, и добавьте
информативное сообщение; ссылки: getParseInfo(), getInterpreter(),
ProfilingATNSimulator, ParseInfo.
🧹 Nitpick comments (2)
src/main/java/org/antlr/v4/runtime/Parser.java (2)
511-517:UnsupportedOperationException— не совсем подходящее исключение для неинициализированного состояния.
UnsupportedOperationExceptionобычно означает, что операция не поддерживается. Здесь же проблема в том, что парсер находится в невалидном состоянии.IllegalStateExceptionбыл бы точнее.Предлагаемое исправление
- throw new UnsupportedOperationException("Parser not initialized"); + throw new IllegalStateException("Parser not initialized");
169-178: Флагcreated— работоспособный, но хрупкий механизм.Комментарий (строки 170-172) честно описывает это как «костыль». Паттерн с
final boolean+ порядком вызовов в конструкторе работает, но неочевиден для читателя:reset()вызывается когдаcreatedещёfalse, а все последующие вызовыreset()уже идут сcreated=true.Рекомендую как минимум добавить краткий комментарий в
reset()рядом сif (created), чтобы поведение было очевидным без необходимости изучать конструктор.
| @Override | ||
| @Nullable | ||
| public ParseInfo getParseInfo() { | ||
| var interp = getInterpreter(); | ||
| if (interp instanceof ProfilingATNSimulator parseInfo) { | ||
| return new ParseInfo(parseInfo); | ||
| } | ||
| return null; | ||
| throw new ClassCastException("Incorrect getInterpreter()"); | ||
| } |
There was a problem hiding this comment.
ClassCastException — неподходящий тип исключения.
Здесь нет фактического приведения типов. ClassCastException семантически неверен и может ввести в заблуждение при отладке. Более корректным будет UnsupportedOperationException или IllegalStateException.
Также обратите внимание, что ранее метод мог возвращать null, а теперь выбрасывает исключение — это ломающее изменение для вызывающего кода.
Предлагаемое исправление
- throw new ClassCastException("Incorrect getInterpreter()");
+ throw new UnsupportedOperationException("getParseInfo() requires a ProfilingATNSimulator interpreter");🤖 Prompt for AI Agents
In `@src/main/java/org/antlr/v4/runtime/Parser.java` around lines 911 - 918, Метод
getParseInfo использует instanceof ProfilingATNSimulator но при несоответствии
кидает ClassCastException — семантически неверно и ломает обратную совместимость
(раньше мог возвращать null). Исправьте реализацию в getParseInfo: если
getInterpreter() не является ProfilingATNSimulator, восстановите поведение назад
и верните null (чтобы не ломать вызывающий код); если вы действительно хотите
выбрасывать исключение, замените ClassCastException на более подходящий тип,
например IllegalStateException или UnsupportedOperationException, и добавьте
информативное сообщение; ссылки: getParseInfo(), getInterpreter(),
ProfilingATNSimulator, ParseInfo.
|
1 participant
No description provided.