Page MenuHomePhabricator

[lint] add the flynt linter for python f-strings
ClosedPublic

Authored by PiRK on Mar 3 2023, 13:46.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC1c7a742fd701: [lint] add the flynt linter for python f-strings
Summary

Convert Python code from "%-formatted" and .format(...) strings into f-strings

Remove existing conversion script that only handles %-formatted strings.

Performance impact:

Before:

$ time arc lint --everything
real	2m15,395s
user	9m59,264s
sys	1m8,720s

After:

$ time arc lint --everything
real	2m14,881s
user	10m6,171s
sys	1m8,421s
Test Plan

Add a few %-formatted "...".format() strings in a functional test, and check that arc lint fixes them.

arc lint --everything

Event Timeline

fix indentation in PHP code

fix more lint errors and warnings (long line, comments style...)

PiRK edited the summary of this revision. (Show Details)

fix stuff (fix extra newline added by flynt to stdout, remove the old string formatting linter which interferes with the new one)
Update the minimum version to the next version.

Fabien added inline comments.
arcanist/linter/FlyntLinter.php
108 ↗(On Diff #38278)

if ($err != 0)

119 ↗(On Diff #38278)

Another (imo cleaner) option:

$stdout = rtrim($stdout) + "\n";

use --stdout option (available in new release 0.78) instead of passing data to stdin to trigger this behavior

Simplify trimming of extra newlines.
Simplify checking for error code.

PiRK published this revision for review.Mar 27 2023, 12:04
Fabien requested changes to this revision.Mar 27 2023, 15:39

This is missing a few things:

  • The summary needs to be updated
  • The CONTRIBUTING document needs to be updated to instruct the users that a new dependency is needed

Question: how much duration time does it adds/removes to arc lint vs the previous linter ?

This revision now requires changes to proceed.Mar 27 2023, 15:39

update contributing.md and install-dependencies-bullseye.sh

The summary has been updated to show timing data before and after. The new tool is neither significantly faster nor slower than the previous script.

This revision is now accepted and ready to land.Mar 28 2023, 12:17