Skip to content

Implement more lexer diagnostics #1245

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

Merged
merged 11 commits into from
Feb 6, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jan 18, 2023

This implements a few more lexer diagnostics.

Each commit of this PR should be standalone and should be reviewable on its own. But since they all build on top of each other, I can’t really create separate PRs for them.

@ahoppen ahoppen force-pushed the ahoppen/99-lexing-backlog branch from 8db5890 to 0fec4fc Compare January 28, 2023 15:51
@ahoppen ahoppen changed the title WIP: Backlog of lexer changes Implement more lexer diagnostics Jan 28, 2023
@ahoppen ahoppen marked this pull request as ready for review January 28, 2023 15:54
@ahoppen ahoppen requested review from rintaro and bnbarham January 28, 2023 15:54
@ahoppen
Copy link
Member Author

ahoppen commented Jan 28, 2023

@swift-ci Please test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@@ -474,6 +474,9 @@ extension FixItMessage where Self == StaticParserFixIt {
public static var replaceCurlyQuoteByNormalQuote: Self {
.init(#"replace curly quotes by '"'"#)
}
public static var replaceNonBreakingSpaceBySpace: Self {
.init("replace non-breaking space by ' '")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by -> with? Same above this one as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I would have always said by but Google agrees that with is preferred. This also changes the ReplaceTokensFixIt, so I’ll do it in a follow-up PR.

@ahoppen ahoppen force-pushed the ahoppen/99-lexing-backlog branch 2 times, most recently from 7474459 to 6c9b223 Compare February 1, 2023 14:07
@ahoppen
Copy link
Member Author

ahoppen commented Feb 1, 2023

@swift-ci Please test

let error = result.error.map { error in
return LexerError(error.kind, byteOffset: cursor.distance(to: error.position))
}
error = error ?? result.error.map { LexerError($0.kind, byteOffset: cursor.distance(to: $0.position)) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a conversion method on LexingError that takes the cursor?

struct LexingError {
  ...
  func toLexerError(_ initialCursor: Cursor) {
    return LexerError(kind, initialCursor.distance(to: position)
  }
}
...
error = error ?? result.error?.toLexerError(cursor)

@ahoppen ahoppen force-pushed the ahoppen/99-lexing-backlog branch from 6c9b223 to 3f7b8a6 Compare February 2, 2023 07:33
@ahoppen
Copy link
Member Author

ahoppen commented Feb 2, 2023

swiftlang/swift#63372

@swift-ci Please test

@ahoppen ahoppen force-pushed the ahoppen/99-lexing-backlog branch from 6a28005 to 139f9df Compare February 2, 2023 15:53
@ahoppen
Copy link
Member Author

ahoppen commented Feb 2, 2023

swiftlang/swift#63372

@swift-ci Please test

@ahoppen ahoppen merged commit ae0da24 into swiftlang:main Feb 6, 2023
@ahoppen ahoppen deleted the ahoppen/99-lexing-backlog branch February 6, 2023 12:08
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Feb 7, 2023
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.

2 participants