From 3f4bb55e07718526fa3bff59d1b48f88d61bdcb5 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Sat, 20 Jan 2018 17:04:52 -0500 Subject: [PATCH] Write a Rubocop Cop to ensure no `#p` or `#puts` calls get committed to master. (#6615) Merge pull request 6615 --- .rubocop.yml | 8 ++++++++ jekyll.gemspec | 2 +- lib/jekyll/commands/help.rb | 4 ++-- lib/jekyll/site.rb | 2 +- rubocop/jekyll.rb | 5 +++++ rubocop/jekyll/no_p_allowed.rb | 23 +++++++++++++++++++++++ rubocop/jekyll/no_puts_allowed.rb | 23 +++++++++++++++++++++++ test/test_filters.rb | 1 - 8 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 rubocop/jekyll.rb create mode 100644 rubocop/jekyll/no_p_allowed.rb create mode 100644 rubocop/jekyll/no_puts_allowed.rb diff --git a/.rubocop.yml b/.rubocop.yml index 969c065f..09e774fd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,4 +1,12 @@ --- + +require: + - ./rubocop/jekyll + +Jekyll/NoPutsAllowed: + Exclude: + - rake/*.rake + AllCops: TargetRubyVersion: 2.1 Include: diff --git a/jekyll.gemspec b/jekyll.gemspec index b434881c..e00cdb00 100644 --- a/jekyll.gemspec +++ b/jekyll.gemspec @@ -22,7 +22,7 @@ Gem::Specification.new do |s| s.homepage = "https://github.com/jekyll/jekyll" all_files = `git ls-files -z`.split("\x0") - s.files = all_files.grep(%r!^(exe|lib)/|^.rubocop.yml$!) + s.files = all_files.grep(%r!^(exe|lib|rubocop)/|^.rubocop.yml$!) s.executables = all_files.grep(%r!^exe/!) { |f| File.basename(f) } s.bindir = "exe" s.require_paths = ["lib"] diff --git a/lib/jekyll/commands/help.rb b/lib/jekyll/commands/help.rb index 42a8017a..80c80e9a 100644 --- a/lib/jekyll/commands/help.rb +++ b/lib/jekyll/commands/help.rb @@ -12,9 +12,9 @@ module Jekyll c.action do |args, _| cmd = (args.first || "").to_sym if args.empty? - puts prog + Jekyll.logger.info prog.to_s elsif prog.has_command? cmd - puts prog.commands[cmd] + Jekyll.logger.info prog.commands[cmd].to_s else invalid_command(prog, cmd) abort diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 2af8e09e..9510bb4c 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -77,7 +77,7 @@ module Jekyll end def print_stats - puts @liquid_renderer.stats_table + Jekyll.logger.info @liquid_renderer.stats_table end # Reset Site details. diff --git a/rubocop/jekyll.rb b/rubocop/jekyll.rb new file mode 100644 index 00000000..31236b8a --- /dev/null +++ b/rubocop/jekyll.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +Dir[File.join(File.expand_path("jekyll", __dir__), "*.rb")].each do |ruby_file| + require ruby_file +end diff --git a/rubocop/jekyll/no_p_allowed.rb b/rubocop/jekyll/no_p_allowed.rb new file mode 100644 index 00000000..cc7d997b --- /dev/null +++ b/rubocop/jekyll/no_p_allowed.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "rubocop" + +module RuboCop + module Cop + module Jekyll + class NoPAllowed < Cop + MSG = "Avoid using `p` to print things. Use `Jekyll.logger` instead.".freeze + + def_node_search :p_called?, <<-PATTERN + (send _ :p _) + PATTERN + + def on_send(node) + if p_called?(node) + add_offense(node, :location => :selector) + end + end + end + end + end +end diff --git a/rubocop/jekyll/no_puts_allowed.rb b/rubocop/jekyll/no_puts_allowed.rb new file mode 100644 index 00000000..a666aacb --- /dev/null +++ b/rubocop/jekyll/no_puts_allowed.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "rubocop" + +module RuboCop + module Cop + module Jekyll + class NoPutsAllowed < Cop + MSG = "Avoid using `puts` to print things. Use `Jekyll.logger` instead.".freeze + + def_node_search :puts_called?, <<-PATTERN + (send nil? :puts _) + PATTERN + + def on_send(node) + if puts_called?(node) + add_offense(node, :location => :selector) + end + end + end + end + end +end diff --git a/test/test_filters.rb b/test/test_filters.rb index afedf746..bd782ea8 100644 --- a/test/test_filters.rb +++ b/test/test_filters.rb @@ -779,7 +779,6 @@ class TestFilters < JekyllUnitTest should "include the size of each grouping" do grouping = @filter.group_by(@filter.site.pages, "layout") grouping.each do |g| - p g assert_equal( g["items"].size, g["size"],