[`pylint`] Implement `missing-maxsplit-arg` (`PLC0207`) #17454

vjurczenia · 2025-04-17T22:09:17Z

Summary

Implements use-maxsplit-arg (PLC0207)
https://pylint.readthedocs.io/en/latest/user_guide/messages/convention/use-maxsplit-arg.html

Emitted when accessing only the first or last element of str.split().
The first and last element can be accessed by using str.split(sep, maxsplit=1)[0] or str.rsplit(sep, maxsplit=1)[-1] instead.

This is part of #970

Test Plan

cargo test

Additionally compared Ruff output to Pylint:

pylint --disable=all --enable=use-maxsplit-arg crates/ruff_linter/resources/test/fixtures/pylint/missing_maxsplit_arg.py

cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/pylint/missing_maxsplit_arg.py --no-cache --select PLC0207

Implements `use-maxsplit-arg` (`PLC0207`) https://pylint.readthedocs.io/en/latest/user_guide/messages/convention/use-maxsplit-arg.html > Emitted when accessing only the first or last element of str.split(). The first and last element can be accessed by using str.split(sep, maxsplit=1)[0] or str.rsplit(sep, maxsplit=1)[-1] instead.

Skylion007 · 2025-04-18T16:13:41Z

Does the original pylint handle slicing too (ie calling split but only using the first n elements)? If not, that would be a great generalization of this rule, maybe one that can be made ruff specific

vjurczenia · 2025-04-18T18:29:03Z

No, it only checks for indexes -1 and 0, but making it more general is a very neat idea.

Having looked into the Pylint source just now, I also realize that this PR is missing the following functionality:

checking the value of a variable used as the index
checking whether a variable used as the index might be mutated at runtime

I'm happy to add this functionality to stay aligned with Pylint and would appreciate maintainer input on whether or not to implement the generalized rule.

crates/ruff_linter/src/codes.rs

ntBre · 2025-04-18T18:38:01Z

I think making the rule more general (both by handling slices and by handling variables) sounds nice, but it's also okay to add a simpler form of a rule first and extend it in follow-up PRs.

github-actions · 2025-04-18T18:43:20Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+47 -0 violations, +0 -0 fixes in 14 projects; 41 projects unchanged)

PlasmaPy/PlasmaPy (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ noxfile.py:445:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

apache/airflow (+9 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ dev/backport/update_backport_status.py:78:21: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ dev/breeze/src/airflow_breeze/commands/common_options.py:442:26: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ dev/breeze/src/airflow_breeze/commands/release_management_commands.py:2625:50: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ dev/breeze/src/airflow_breeze/prepare_providers/provider_documentation.py:658:34: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ dev/breeze/src/airflow_breeze/utils/run_utils.py:118:25: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ dev/breeze/src/airflow_breeze/utils/version_utils.py:44:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ providers/amazon/tests/system/amazon/aws/example_bedrock.py:66:24: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ providers/amazon/tests/system/amazon/aws/example_glue.py:76:12: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ scripts/in_container/install_airflow_and_providers.py:32:21: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

apache/superset (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ tests/common/query_context_generator.py:214:29: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ tests/common/query_context_generator.py:255:22: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

bokeh/bokeh (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ src/bokeh/io/notebook.py:663:26: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ src/bokeh/util/token.py:142:41: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ src/bokeh/util/token.py:155:41: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

freedomofpress/securedrop (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ securedrop/store.py:345:49: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

ibis-project/ibis (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ ibis/examples/gen_registry.py:125:52: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

langchain-ai/langchain (+9 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ libs/core/langchain_core/_api/deprecation.py:362:17: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/_api/deprecation.py:362:56: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/_api/deprecation.py:370:17: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/_api/deprecation.py:371:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/_api/deprecation.py:475:24: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/_api/deprecation.py:494:27: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/runnables/graph_mermaid.py:157:24: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/utils/mustache.py:85:19: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ libs/core/langchain_core/utils/utils.py:140:32: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

pandas-dev/pandas (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ pandas/compat/_optional.py:166:14: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

pypa/cibuildwheel (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ bin/update_pythons.py:150:22: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ cibuildwheel/platforms/macos.py:153:31: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ cibuildwheel/selector.py:78:26: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

rotki/rotki (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ rotkehlchen/assets/converters.py:417:17: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

zulip/zulip (+11 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ zerver/lib/send_email.py:287:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/lib/send_email.py:429:21: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/lib/upload/local.py:67:17: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/lib/upload/s3.py:176:25: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/lib/webhooks/common.py:242:26: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/lib/webhooks/common.py:269:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/middleware.py:228:49: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/views/upload.py:374:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/webhooks/gitlab/view.py:46:36: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/webhooks/stripe/view.py:356:39: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ zerver/webhooks/wekan/view.py:15:12: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

wntrblm/nox (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ tests/test_sessions.py:1199:44: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

pytest-dev/pytest (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ src/_pytest/config/findpaths.py:160:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ src/_pytest/terminal.py:1007:40: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`
+ src/_pytest/terminal.py:459:41: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

astropy/astropy (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ astropy/io/ascii/latex.py:443:16: PLC0207 Accessing only the first or last element of `str.split()` without setting `maxsplit=1`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PLC0207	47	47	0	0	0

ntBre · 2025-04-18T18:49:01Z

crates/ruff_linter/src/rules/pylint/rules/missing_maxsplit_arg.rs

+    if keywords.iter().any(|kw| {
+        kw.arg.as_deref() == Some("maxsplit")
+            && matches!(
+                kw.value,
+                ast::Expr::NumberLiteral(ast::ExprNumberLiteral {
+                    value: ast::Number::Int(ast::Int::ONE),
+                    ..
+                })
+            )
+    }) {
+        return;
+    }


The ecosystem check turned up some results like this:

ext = file_path.rsplit(".", 2)[-1].lower()

The maxsplit argument doesn't have to be passed as a keyword. We have a find_argument_value helper that should handle this and also extract the value for your comparison to 1:

ruff/crates/ruff_python_ast/src/nodes.rs

Lines 2854 to 2859 in 1108f66

/// Return the value for the argument with the given name or at the given position, or `None` if no such

/// argument exists. Used to retrieve argument values that can be provided _either_ as keyword or

/// positional arguments.

pub fn find_argument_value(&self, name: &str, position: usize) -> Option<&Expr> {

self.find_argument(name, position).map(ArgOrKeyword::value)

}

Oh I see, you're doing the positional comparison below, and this is a true positive because the maxsplit value also needs to be 1. In any case, this should help to combine the keyword and positional checks!

It's way neater with find_argument_value, so thanks for that!

However, this ecosystem check failure had me re-evaluate the test included, so I compared it to the Pylint one and noticed that maxsplit=2 should be a passing case. The relevant Pylint source is here.

Following that discovery, I've updated the test cases to closer match the Pylint ones, which left me with a few that require additional implementation. These will require identifying:

when split is called on slices of strings

kwargs set via unpacked dicts

if index is set via variable

if the index variable is modified in a loop (that includes the split call)

if split is called on a static class member that happens to be a string (I'm uncertain about this one, but think it should be possible.)

if split is called on a method that returns a string (I'm uncertain that this one is possible.)

As before, any advice is appreciated.

if split is called on a static class member that happens to be a string

I've just managed to figure this one out, and its given me a bit of a confidence boost on the rest of them, so I've converted this PR to a Draft until they're done.

Awesome, thanks for the update and for your work on this. Ping me again if you end up needing help. I'm also happy merging a simpler first draft of the rule that we can expand later, if these end up being trickier than you want to deal with!

If it comes to that, let's just include tests for the unhandled cases with clear TODO comments for future work.

Hi @ntBre , hope all is well. Sorry for the delay on getting back to this.

As you can see from my most recent commits, I've managed to get through a few of the cases I had listed. However, I think I've hit a bit of a wall with the rest. The remaining ones are:

if index is set via a variable

if the index variable is modified in a loop (that includes the split call)

if split is called on a method that returns a string

if kwargs are set via an unpacked dict variable

I've documented these as TODO in the test, and am happy to go ahead with merging this simpler first draft if you are.

…dule

ntBre

Thanks, this is looking great! I think this would be a good place to finish this PR and leave other extensions as follow-ups, as you said. It could be fun to add an auto-fix for this rule, as another follow-up, if you're interested :)

crates/ruff_linter/src/rules/pylint/rules/missing_maxsplit_arg.rs

crates/ruff_linter/resources/test/fixtures/pylint/missing_maxsplit_arg.py

ntBre

Thank you, this looks great!

I had two minor nits but this is otherwise good to go.

crates/ruff_linter/src/rules/pylint/rules/missing_maxsplit_arg.rs

crates/ruff_linter/resources/test/fixtures/pylint/missing_maxsplit_arg.py

ntBre

Thanks again, this is great!

ntBre · 2025-05-28T20:39:00Z

I merged main to pick up our rust version updates and then ran cargo fmt to make CI happy

ntBre added the rule Implementing or modifying a lint rule label Apr 18, 2025

ntBre reviewed Apr 18, 2025

View reviewed changes

crates/ruff_linter/src/codes.rs Outdated Show resolved Hide resolved

ntBre added the preview Related to preview mode features label Apr 18, 2025

ntBre reviewed Apr 18, 2025

View reviewed changes

vjurczenia added 7 commits April 19, 2025 12:04

Move rule to preview group

38a9c81

Use find_argument_value to check maxsplit value

347e9b9

Refactor to import required items from ast module instead of whole mo…

5bec68b

…dule

cargo fmt

3595079

Update tests

e6e2bfe

Remove check of maxsplit value (aligned with Pylint)

1f34119

Add functionality to check class members of type string

df38ac6

vjurczenia marked this pull request as draft April 23, 2025 00:56

vjurczenia added 8 commits April 24, 2025 19:18

Remove unnecessary assignments from test

9705675

Add functionality to check sliced strings

1d3502c

Update snapshot

8967bc8

Enhance sliced strings check to include chained slices

a61c622

Add tests for class attribute named split (from Pylint)

7342bbc

Add functionality to check kwargs set via an unpacked dict literal

d1bb448

Remove incorrect comments from test file

dfbde3e

Add remaining test cases

5053354

vjurczenia marked this pull request as ready for review May 12, 2025 21:22

ntBre mentioned this pull request May 14, 2025

SIM905 fix is incorrect for unpacked dict arguments #18101

Open

ntBre reviewed May 14, 2025

View reviewed changes

vjurczenia added 2 commits May 15, 2025 23:23

Action comments

b67bc21

Remove incorrect hyphen from comment

77cd518

Add TODO test comments

378de24

ntBre approved these changes May 19, 2025

View reviewed changes

crates/ruff_linter/src/rules/pylint/rules/missing_maxsplit_arg.rs Outdated Show resolved Hide resolved

crates/ruff_linter/resources/test/fixtures/pylint/missing_maxsplit_arg.py Outdated Show resolved Hide resolved

Action comments

5997aa7

ntBre approved these changes May 28, 2025

View reviewed changes

ntBre added 2 commits May 28, 2025 16:37

Merge branch 'main' into vjurczenia/missing_maxsplit_arg

f7f6fbb

cargo fmt

98a0f50

tidy up after merge

ab261cb

ntBre changed the title ~~[pylint] Implement missing-maxsplit-arg (PLC0207)~~ [pylint] Implement missing-maxsplit-arg (PLC0207) May 28, 2025

ntBre enabled auto-merge (squash) May 28, 2025 20:44

ntBre merged commit 27743ef into astral-sh:main May 28, 2025
32 checks passed

BrewTestBot mentioned this pull request May 29, 2025

ruff 0.11.12 Homebrew/homebrew-core#225103

Merged

	/// Return the value for the argument with the given name or at the given position, or `None` if no such
	/// argument exists. Used to retrieve argument values that can be provided _either_ as keyword or
	/// positional arguments.
	pub fn find_argument_value(&self, name: &str, position: usize) -> Option<&Expr> {
	self.find_argument(name, position).map(ArgOrKeyword::value)
	}

[pylint] Implement missing-maxsplit-arg (PLC0207) #17454

[pylint] Implement missing-maxsplit-arg (PLC0207) #17454

Conversation

vjurczenia commented Apr 17, 2025

Summary

Test Plan

Uh oh!

Skylion007 commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vjurczenia commented Apr 18, 2025

Uh oh!

Uh oh!

ntBre commented Apr 18, 2025

Uh oh!

github-actions bot commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

ntBre Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

vjurczenia Apr 20, 2025

Choose a reason for hiding this comment

Uh oh!

vjurczenia Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

vjurczenia May 12, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!

[`pylint`] Implement `missing-maxsplit-arg` (`PLC0207`) #17454

[`pylint`] Implement `missing-maxsplit-arg` (`PLC0207`) #17454

Skylion007 commented Apr 18, 2025 •

edited

Loading

github-actions bot commented Apr 18, 2025 •

edited

Loading

`ruff-ecosystem` results