# How to fix your code by coding the fixer Kang-min Liu // @gugod LINE Fukuoka TPC::Glasgow ---- Coding, by coding the coder. --- ## My team - 3 frontend (js) devs, 4 server-side (perl) devs - I'm the newbie. - code review + human QA before rollout - 30% time on code review. --- ## Code review pros - Requires understanding of spec and code - Ensure "readability" - increase "maintainability" Note: I'm not defining "readability" or "maintainability" here. Let's just say: Being able to understand other's code should also enables you to maintain those code. --- ## Readability - Unconventional - prefer tech X for problem Y - Quirks, magic, shenanigans - "Why does it need to be done like this?" ---- ## Readability - Mismatching style - "This does not looks pretty" - Random "code waste" - Unused stuff --- ## Style - Dress - "Fashion" - Shape & Color - Whitespace management Note: Why using "LaTeX" instead of $whatever ? How many time you've spent on configuring the fonts & color of your Termnial / Editor? You probably dislike Comic Sans font for a reason. ---- - Perl::Tidy - $EDITOR ---- ## Code Wastes - Unused variables - Unused subroutines - Unused imports - Uncleaned namespace - Unused hash key/value --- ## Tools to swipe code wastes ---- - Perl::Critic - App::Critique - Ref::Util::Rewriter ---- - eslint https://eslint.org/ - gofix https://golang.org/cmd/fix/ - Fix finds Go programs that use old APIs and rewrites them to use newer ones. After you update to a new Go release, fix helps make the necessary changes to your programs - search: code refactoring tool --- # PPI ---- ```perl $found = $doc->find(...) or return; @wastes = grep { ... } @$found; $_->remove for @wastes; ``` Deleting stuff ---- ```perl $elem2 = ... $elem->insert_before( $elem2 ); $elem->insert_after( $elem2 ); ``` Adding stuff ---- ```perl my $new_statement = ...; $elem->insert_before( $new_statement ); $elem->insert_before( PPI::Token::Whitespace->new("\n") ); ``` Adding a new statement. ---- ```perl my $doc2 = PPI::Document->new(\$code_str); my @elems = $doc2->children; $_->remove for @elems; my $e = $doc->find_first(...); $e->insert_before($_) for @elems; ``` Paste $code_str before $e Note: PPI::Document::Fragement exists but it is not really integrated with rest of PPI ---- ```perl $elem->insert_before($_) for @new_stuff; $elem->remove; ``` Replace one element with others --- ## p5-nitpick https://github.com/gugod/p5-nitpick Finds smelly spots and remove them. ---- Removing unused variables ```perl my $o = App::P5Nitpick::PCPWrap->new( 'Perl::Critic::Policy::Variables::ProhibitUnusedVariables' ); my @vio = $o->violates( undef, Perl::Critic::Document->new(-source => $doc) ); for (@vio) { my ($msg, $explain, $el) = @$_; if ($el->variables == 1) { $el->remove; } } ``` ---- ```perl my $foo; # my $foo; is removed my ($bar, $qux); # $qux is not removed $bar = ... $bar->do_something; ... ``` ---- Remove unused include statement. ``` use Try::Tiny; # unused use HTTP::Tiny; my $res = HTTP::Tiny->get(...); do_something($res); ``` ---- ```perl my $has_try_block = 0; for my $try_keyword (@{ $doc->find(sub { $_[1]->isa('PPI::Token::Word') && $_[1]->content eq 'try' }) ||[] }) { my $try_block = $try_keyword->snext_sibling or next; next unless $try_block->isa('PPI::Structure::Block'); $has_try_block = 1; last; } return if $has_try_block; my @includes = @{ $doc->find('PPI::Statement::Include') ||[] }; return unless @includes; for my $try_module (qw(Try::Tiny Try::Catch Try::Lite TryCatch Try)) { my @uses = grep { $_->module eq $try_module } @includes; if (@uses && !$has_try_block) { push @violations, map { $self->violation("Unused ${try_module} module", 'There are no `try` block in the code.', $_) } @uses; } } ``` ---- AppendUnimportStatement ```perl use Moose; ... no Moose; # ← 1; ``` for Moose, Mouse, Moo, Moose::Role, Mouse::Role ---- ```perl my $doc2 = PPI::Document->new(\"no Moose;"); my $el = $doc2->find_first('PPI::Statement::Include'); $el->remove; my $c = $doc->schild(-1); # '1;' $c->insert_before($el); $c->insert_before(PPI::Token::Whitespace->new("\n")); ``` ---- ```perl my $doc2 = PPI::Document->new(\"use namespace::autoclean;"); my $el = $doc2->find_first('PPI::Statement::Include'); $el->remove; my $c = $doc->schild(2); $c->insert_before($el); $c->insert_before(PPI::Token::Whitespace->new("\n")); ``` Insert `use namespace::autoclean;` right after `package` statement. ---- RewriteWithAssignmentOperators ```perl # From $a = $a + 3; #To $a += 3; ``` ---- ``` > ppi-dump -e '$a = $a + 3;' | grep -v Whitespace PPI::Document PPI::Statement PPI::Token::Symbol '$a' PPI::Token::Operator '=' PPI::Token::Symbol '$a' PPI::Token::Operator '+' PPI::Token::Number '3' PPI::Token::Structure ';' ``` ---- ```perl my @found = ... grep { $_->schildren == 6 } @{ $document->find('PPI::Statement') ||[] }; ``` ``` $a = $a + 3 ; 1 2 3 4 5 6 ``` ---- ```perl my @found= ... grep { my $c = $_->schild(1); $c->isa('PPI::Token::Operator') && $c->content eq '=' } ... @{ $document->find('PPI::Statement') ||[] }; ``` <pre><code>$a <span style="color:red">=</span> $a + 3 ;</code></pre> ---- ```perl my @found= ... grep { my $c = $_->schild(3); $c->isa('PPI::Token::Operator') && $c->content !~ m{\A( -> | > | < )\z}x; } ... @{ $document->find('PPI::Statement') ||[] }; ``` <pre><code> $a = $a <span style="color:red">+</span> 3 ; $a = $a <span style="color:red">&gt;</span> 3; $a = $a <span style="color:red">&lt;</span> 3; $a = $a <span style="color:red">-&gt;</span>foobar; </code></pre> ---- ```perl my @found= ... grep { my $c1 = $_->schild(0); my $c2 = $_->schild(2); $c1->isa('PPI::Token::Symbol') && $c1->raw_type eq '$' && $c2->isa('PPI::Token::Symbol') && $c2->raw_type eq '$' && $c1->content && $c2->content } ... @{ $document->find('PPI::Statement') ||[] }; ``` <pre><code><span style="color:red">$a</span> = <span style="color:red">$a</span> + 3 ;</code></pre> ---- ```perl for (@found) { my @child = $_->schildren; # $a = $a + 3 ; # 0 1 2 3 4 5 # '=' , '+' => '+=' my $op = PPI::Token::Operator->new($child[3]->content . $child[1]->content); $child[3]->remove; # '+' $child[2]->remove; # '$a' # '=' => '+=' $child[1]->insert_after($op); $child[1]->remove; } ``` --- ## Perl::Critic::TooMuchCode - ProhibitUnusedConstant - ProhibitUnusedImport - ProhibitUnusedInclude - ProhibitUnnecessaryUTF8Pragma - ProhibitLargeBlock - ProhibitLargeTryBlock https://metacpan.org/pod/Perl::Critic::TooMuchCode --- ### More code-fixable patterns - `$n = scalar @arr;` - `do_this();;` Extra semi-colons / empty statements. - `@a = @{[ "foo", "bar", foo() ]}` - Unused hash key/value - typo in Document - typo in variable name (maybe) --- ## Wrap-up - Altering code with PPI is generally easy. - Finding what can be altered can be tedious - Check Perl::Critic::Utils - Identifying bad tastes is difficult but seems to be worthwhile --- ## References - [p5-nitpick](https://github.com/gugod/p5-nitpick) - [PPI::Transform](https://metacpan.org/pod/PPI::Transform) - [Devel::Mutator](https://metacpan.org/pod/Devel::Mutator) - [PPIx::EditorTools](https://metacpan.org/pod/PPIx::EditorTools) - [Devel::PerlySense](https://metacpan.org/pod/Devel::PerlySense) --- "Thank you for listening";
{"metaMigratedAt":"2023-06-14T17:26:18.870Z","metaMigratedFrom":"YAML","title":"How to fix your code by coding the fixer","breaks":true,"slideOptions":"{\"transition\":\"none\"}","contributors":"[{\"id\":\"dcce249b-7b0f-4093-b97b-479c7762dfba\",\"add\":16836,\"del\":9186}]"}
    1657 views