Skip to content

Conversation

TaKO8Ki
Copy link
Contributor

@TaKO8Ki TaKO8Ki commented Sep 1, 2025

Summary

Fixes #19887

  • flynt(FLY002): When joining only string constants, upgrade raw single-quoted strings to raw triple-quoted if the resulting
    content contains a newline.
  • Choose a safe triple-quote delimiter by switching to the opposite quote style if the preferred triple appears inside the
    content.
  • Update FLY002 snapshot to include the \n'.join([r'line1','line2']) case.

Test Plan

I've added one test case to FLY002.py.

Copy link
Contributor

github-actions bot commented Sep 1, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks! I think we can simplify a bit, and there are also some edge cases we still need to handle.

flags = flags.with_triple_quotes(ruff_python_ast::str::TripleQuotes::Yes);

// Ensure we pick a safe triple-quote delimiter that doesn't occur in the content.
let quote = flags.quote_style();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could use a combination of StringFlags::quote_str and Quote::opposite to simplify this section a bit.

Should we add a test case (or a few test cases) for this behavior?

I also think we may need to check for both ''' and """ in every string. We could end up with a pathological case like this:

'\n'.join([r"raw string", '<""">', "<'''>"])

which we can't join into a single string with either delimiter. I think if we have a raw string, and the joined string contains our attempted triple quotes, we'll just have to bail out without a fix.

CPython escapes some of the quotes in this case:

>>> '\n'.join([r"raw string", '<""">', "<'''>"])
'raw string\n<""">\n<\'\'\'>'

I think we could also do that with our UnicodeEscape type, but I'd be fine with, or even prefer, avoiding the fix in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the suggestion. I have addressed your comment.

@TaKO8Ki TaKO8Ki requested a review from ntBre September 11, 2025 18:43
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.

FLY002 autofix with raw strings causes a syntax error
2 participants