Allow `NUL` character in f-strings by dhruvmanila · Pull Request #7378 · astral-sh/ruff

dhruvmanila · 2023-09-14T07:24:24Z

Summary

This PR fixes the bug to allow NUL (\0) character inside f-strings.

Test Plan

Add test case with NUL character inside f-string.

dhruvmanila · 2023-09-14T07:24:35Z

Current dependencies on/for this PR:

main
- PR Add support for PEP 701 #7376

This comment was auto-generated by Graphite.

dhruvmanila · 2023-09-14T07:25:51Z

crates/ruff_python_parser/src/lexer.rs

+                // The condition is to differentiate between the `NUL` (`\0`) character
+                // in the source code and the one returned by `self.cursor.first()` when
+                // we reach the end of the source code.
+                EOF_CHAR if self.cursor.is_eof() => {


We could move this in the catch-all branch but that would mean that the check will be performed for almost every character.

Hmm yeah. That's why using self.cursor.first() is "dangerous" and I prefer to use bump wherever I can. Can you do a quick double check if any other code needs to check for self.cursor.is_eof()

Yeah, that's the only place where it seems necessary.

MichaReiser · 2023-09-14T08:47:17Z

crates/ruff_python_parser/src/lexer.rs

+                // The condition is to differentiate between the `NUL` (`\0`) character
+                // in the source code and the one returned by `self.cursor.first()` when
+                // we reach the end of the source code.
+                EOF_CHAR if self.cursor.is_eof() => {


Hmm yeah. That's why using self.cursor.first() is "dangerous" and I prefer to use bump wherever I can. Can you do a quick double check if any other code needs to check for self.cursor.is_eof()

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

Allow NUL character in f-strings

0f21f07

dhruvmanila mentioned this pull request Sep 14, 2023

Add support for PEP 701 #7376

Merged

dhruvmanila commented Sep 14, 2023

View reviewed changes

dhruvmanila added parser Related to the parser python312 Related to Python 3.12 labels Sep 14, 2023

dhruvmanila requested a review from MichaReiser September 14, 2023 07:26

MichaReiser approved these changes Sep 14, 2023

View reviewed changes

dhruvmanila merged commit 5d0063a into dhruv/pep-701 Sep 14, 2023

dhruvmanila deleted the dhruv/fstring-nul branch September 14, 2023 10:58

dhruvmanila mentioned this pull request Sep 15, 2023

Add support for PEP 701 in the parser #7043

Closed

dhruvmanila linked an issue Sep 15, 2023 that may be closed by this pull request

Add support for PEP 701 in the lexer #7042

Closed

dhruvmanila added a commit that referenced this pull request Sep 18, 2023

Allow NUL character in f-strings (#7378)

93d2e0c

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila added a commit that referenced this pull request Sep 19, 2023

Allow NUL character in f-strings (#7378)

ea6f32c

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila mentioned this pull request Sep 19, 2023

Update ISC001, ISC002 to check in f-strings #7515

Merged

dhruvmanila added a commit that referenced this pull request Sep 20, 2023

Allow NUL character in f-strings (#7378)

afa2669

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila added a commit that referenced this pull request Sep 22, 2023

Allow NUL character in f-strings (#7378)

777a217

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila added a commit that referenced this pull request Sep 22, 2023

Allow NUL character in f-strings (#7378)

d01dbf5

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

This was referenced Sep 22, 2023

Use the new f-string tokens in string formatting #7586

Merged

Separate Q003 to accomodate f-string context #7588

Merged

Update Q000, Q001 with the new f-string tokens #7589

Merged

dhruvmanila added a commit that referenced this pull request Sep 22, 2023

Allow NUL character in f-strings (#7378)

1e0b304

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila mentioned this pull request Sep 22, 2023

Ignore quote escapes in expression part of f-string #7597

Merged

dhruvmanila added a commit that referenced this pull request Sep 26, 2023

Allow NUL character in f-strings (#7378)

ceb127c

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila mentioned this pull request Sep 26, 2023

Remove StringKind::FString and StringKind::RawFString #7667

Merged

dhruvmanila added a commit that referenced this pull request Sep 27, 2023

Allow NUL character in f-strings (#7378)

6997940

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila added a commit that referenced this pull request Sep 28, 2023

Allow NUL character in f-strings (#7378)

e6f36b9

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila mentioned this pull request Sep 28, 2023

Update logical line rules for the new f-string tokens #7690

Merged

dhruvmanila added a commit that referenced this pull request Sep 29, 2023

Allow NUL character in f-strings (#7378)

3f81cb1

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

dhruvmanila added a commit that referenced this pull request Sep 29, 2023

Allow NUL character in f-strings (#7378)

846948f

## Summary This PR fixes the bug to allow `NUL` (`\0`) character inside f-strings. ## Test Plan Add test case with `NUL` character inside f-string.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `NUL` character in f-strings#7378

Allow `NUL` character in f-strings#7378
dhruvmanila merged 1 commit intodhruv/pep-701from
dhruv/fstring-nul

dhruvmanila commented Sep 14, 2023

Uh oh!

dhruvmanila commented Sep 14, 2023 •

edited

Loading

Uh oh!

dhruvmanila Sep 14, 2023

Uh oh!

MichaReiser Sep 14, 2023

Uh oh!

dhruvmanila Sep 14, 2023

Uh oh!

MichaReiser Sep 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dhruvmanila commented Sep 14, 2023

Summary

Test Plan

Uh oh!

dhruvmanila commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhruvmanila Sep 14, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser Sep 14, 2023

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Sep 14, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser Sep 14, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dhruvmanila commented Sep 14, 2023 •

edited

Loading