Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix line accounting for alternateCommentMode #2018

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shicks
Copy link

@shicks shicks commented Aug 18, 2024

When alternateCommentMode is enabled, the tokenizer treats a block of multiple end-of-line (//) comments as a single line when keeping track of line numbers, making error messages very difficult to understand, since they refer to the wrong line number.

This change increments the line number in the inner loop, so that each line is accounted correctly.

To reproduce the bug:

$ npm install protobufjs

$ cat > broken.proto <<EOF
syntax = "proto3";

// Multiple lines of
// end-of-line comments
// seem to mess up
// the tokenizer's
// bookkeeping.

message Foo {};a
EOF

$ cat > load.js <<EOF
const {Root} = require('protobufjs');
new Root().load('broken.proto', {alternateCommentMode: true});
EOF

$ node load.js

This reports an error at broken.js line 5, rather than line 9 where the error actually is.

When `alternateCommentMode` is enabled, the tokenizer treats a block of multiple end-of-line (`//`) comments as a single line when keeping track of line numbers, making error messages very difficult to understand, since they refer to the wrong line number.

This change increments the line number in the inner loop, so that each line is accounted correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant