From ee15a462a9a1612e0a5b34af34d0da7410d7d1de Mon Sep 17 00:00:00 2001 From: Jan Piotrowski Date: Tue, 21 Aug 2018 19:05:19 +0200 Subject: [PATCH] Add rubocop to docs project (#679) `Fastfile` didn't follow fastlane standard, quickest fix was to add rubocop to the project. Also found some inconsistencies in other files. --- .rubocop.yml | 311 +++++++++++++++++++++++++++ Gemfile | 6 +- Gemfile.lock | 19 +- fastlane/Fastfile | 25 ++- fastlane/actions/test_sample_code.rb | 2 +- scripts/ci/generate_redirects.rb | 6 +- 6 files changed, 349 insertions(+), 20 deletions(-) create mode 100644 .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000000..f9ef9aa6b2 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,311 @@ +Style/MultipleComparison: + Enabled: false + +Style/PercentLiteralDelimiters: + Enabled: false + +# kind_of? is a good way to check a type +Style/ClassCheck: + EnforcedStyle: kind_of? + +Style/FrozenStringLiteralComment: + Enabled: false + +# This doesn't work with older versions of Ruby (pre 2.4.0) +Style/SafeNavigation: + Enabled: false + +# This doesn't work with older versions of Ruby (pre 2.4.0) +Performance/RegexpMatch: + Enabled: false + +# This suggests use of `tr` instead of `gsub`. While this might be more performant, +# these methods are not at all interchangable, and behave very differently. This can +# lead to people making the substitution without considering the differences. +Performance/StringReplacement: + Enabled: false + +# .length == 0 is also good, we don't always want .zero? +Style/NumericPredicate: + Enabled: false + +# this would cause errors with long lanes +Metrics/BlockLength: + Enabled: false + +# this is a bit buggy +Metrics/ModuleLength: + Enabled: false + +# certificate_1 is an okay variable name +Style/VariableNumber: + Enabled: false + +# This is used a lot across the fastlane code base for config files +Style/MethodMissing: + Enabled: false + +# This rule isn't useful, lots of discussion happening around it also +# e.g. https://github.com/bbatsov/rubocop/issues/2338 +MultilineBlockChain: + Enabled: false + +# +# File.chmod(0777, f) +# +# is easier to read than +# +# File.chmod(0o777, f) +# +Style/NumericLiteralPrefix: + Enabled: false + +# +# command = (!clean_expired.nil? || !clean_pattern.nil?) ? CLEANUP : LIST +# +# is easier to read than +# +# command = !clean_expired.nil? || !clean_pattern.nil? ? CLEANUP : LIST +# +Style/TernaryParentheses: + Enabled: false + +# sometimes it is useful to have those empty methods +Style/EmptyMethod: + Enabled: false + +# It's better to be more explicit about the type +Style/BracesAroundHashParameters: + Enabled: false + +# specs sometimes have useless assignments, which is fine +Lint/UselessAssignment: + Exclude: + - '**/spec/**/*' + +# In specs requires are done automatically +Require/MissingRequireStatement: + Exclude: + - '**/spec/**/*.rb' + - '**/spec_helper.rb' + - 'spaceship/lib/spaceship/babosa_fix.rb' + - '**/Fastfile' + - '**/*.gemspec' + - 'rakelib/**/*' + - '**/*.rake' + - '**/Rakefile' + - 'fastlane/**/*' # TODO: remove + - 'supply/**/*' # TODO: remove + +# We could potentially enable the 2 below: +Layout/IndentHash: + Enabled: false + +Layout/AlignHash: + Enabled: false + +# HoundCI doesn't like this rule +Layout/DotPosition: + Enabled: false + +# We allow !! as it's an easy way to convert to boolean +Style/DoubleNegation: + Enabled: false + +# Prevent to replace [] into %i +Style/SymbolArray: + Enabled: false + +# We still support Ruby 2.0.0 +Layout/IndentHeredoc: + Enabled: false + +# This cop would not work fine with rspec +Style/MixinGrouping: + Exclude: + - '**/spec/**/*' + +# Sometimes we allow a rescue block that doesn't contain code +Lint/HandleExceptions: + Enabled: false + +# Cop supports --auto-correct. +Lint/UnusedBlockArgument: + Enabled: false + +Lint/AmbiguousBlockAssociation: + Enabled: false + +# Needed for $verbose +Style/GlobalVars: + Enabled: false + +# We want to allow class Fastlane::Class +Style/ClassAndModuleChildren: + Enabled: false + +# $? Exit +Style/SpecialGlobalVars: + Enabled: false + +Metrics/AbcSize: + Enabled: false + +Metrics/MethodLength: + Enabled: false + +Metrics/CyclomaticComplexity: + Enabled: false + +# The %w might be confusing for new users +Style/WordArray: + MinSize: 19 + +# raise and fail are both okay +Style/SignalException: + Enabled: false + +# Better too much 'return' than one missing +Style/RedundantReturn: + Enabled: false + +# Having if in the same line might not always be good +Style/IfUnlessModifier: + Enabled: false + +# and and or is okay +Style/AndOr: + Enabled: true + EnforcedStyle: conditionals + +# Configuration parameters: CountComments. +Metrics/ClassLength: + Max: 320 + + +# Configuration parameters: AllowURI, URISchemes. +Metrics/LineLength: + Max: 370 + +# Configuration parameters: CountKeywordArgs. +Metrics/ParameterLists: + Max: 17 + +Metrics/PerceivedComplexity: + Max: 18 + +# Sometimes it's easier to read without guards +Style/GuardClause: + Enabled: false + +# We allow both " and ' +Style/StringLiterals: + Enabled: false + +# something = if something_else +# that's confusing +Style/ConditionalAssignment: + Enabled: false + +# Better to have too much self than missing a self +Style/RedundantSelf: + Enabled: false + +# e.g. +# def self.is_supported?(platform) +# we may never use `platform` +Lint/UnusedMethodArgument: + Enabled: false + +# the let(:key) { ... } +Lint/ParenthesesAsGroupedExpression: + Exclude: + - '**/spec/**/*' + +# This would reject is_ in front of methods +# We use `is_supported?` everywhere already +Style/PredicateName: + Enabled: false + +# We allow the $ +Style/PerlBackrefs: + Enabled: false + +# Disable '+ should be surrounded with a single space' for xcodebuild_spec.rb +Layout/SpaceAroundOperators: + Exclude: + - '**/spec/actions_specs/xcodebuild_spec.rb' + +AllCops: + TargetRubyVersion: 2.0 + Include: + - '*/lib/assets/*Template' + - '*/lib/assets/*TemplateAndroid' + Exclude: + - '**/lib/assets/custom_action_template.rb' + - './vendor/**/*' + - '**/lib/assets/DefaultFastfileTemplate' + +# They have not to be snake_case +Style/FileName: + Exclude: + - '**/Dangerfile' + - '**/Brewfile' + - '**/Gemfile' + - '**/Podfile' + - '**/Rakefile' + - '**/Fastfile' + - '**/Deliverfile' + - '**/Snapfile' + - '**/*.gemspec' + +# We're not there yet +Style/Documentation: + Enabled: false + +# Added after upgrade to 0.38.0 +Style/MutableConstant: + Enabled: false + +# length > 0 is good +Style/ZeroLengthPredicate: + Enabled: false + +# Adds complexity +Style/IfInsideElse: + Enabled: false + +# Sometimes we just want to 'collect' +Style/CollectionMethods: + Enabled: false + +# Check whether fork is handled correctly on all platforms +CrossPlatform/ForkUsage: + Exclude: + - '**/plugins/template/**/*' + +# ( ) for method calls +Style/MethodCallWithArgsParentheses: + Enabled: true + IgnoredMethods: + - 'require' + - 'require_relative' + - 'fastlane_require' + - 'gem' + - 'program' + - 'command' + - 'raise' + - 'attr_accessor' + - 'attr_reader' + - 'desc' + - 'lane' + - 'private_lane' + - 'platform' + # rspec tests code below + - 'to' + - 'describe' + - 'it' + - 'be' + - 'context' + - 'before' + - 'after' diff --git a/Gemfile b/Gemfile index a44b065382..7c3fee6097 100644 --- a/Gemfile +++ b/Gemfile @@ -1,5 +1,7 @@ -source "https://rubygems.org" +source("https://rubygems.org") # We want to test with the latest master branch -gem "fastlane", git: "https://github.com/fastlane/fastlane" gem "danger" +gem "fastlane", git: "https://github.com/fastlane/fastlane" + +gem "rubocop", "0.49.1" diff --git a/Gemfile.lock b/Gemfile.lock index 5c862ce329..e4f5fe4701 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,6 +45,7 @@ GEM CFPropertyList (3.0.0) addressable (2.5.2) public_suffix (>= 2.0.2, < 4.0) + ast (2.4.0) atomos (0.1.3) babosa (1.0.2) claide (1.0.2) @@ -125,15 +126,30 @@ GEM octokit (4.10.0) sawyer (~> 0.8.0, >= 0.5.3) open4 (1.3.4) + parallel (1.12.1) + parser (2.5.1.2) + ast (~> 2.4.0) os (1.0.0) plist (3.4.0) + powerpack (0.1.2) public_suffix (2.0.5) + rainbow (2.2.2) + rake + rake (12.3.1) representable (3.0.4) declarative (< 0.1.0) declarative-option (< 0.2.0) uber (< 0.2.0) retriable (3.1.2) rouge (2.0.7) + rubocop (0.49.1) + parallel (~> 1.10) + parser (>= 2.3.3.1, < 3.0) + powerpack (~> 0.1) + rainbow (>= 1.99.1, < 3.0) + ruby-progressbar (~> 1.7) + unicode-display_width (~> 1.0, >= 1.0.1) + ruby-progressbar (1.10.0) rubyzip (1.2.1) sawyer (0.8.1) addressable (>= 2.3.5, < 2.6) @@ -178,6 +194,7 @@ PLATFORMS DEPENDENCIES danger fastlane! + rubocop (= 0.49.1) BUNDLED WITH - 1.16.2 + 1.16.3 diff --git a/fastlane/Fastfile b/fastlane/Fastfile index 8c49ba3e60..522a7d37d5 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -24,23 +24,23 @@ lane :update_available_plugins do end private_lane :build_site do - UI.message "🕗 Building page..." - sh "mkdocs build --clean --strict" # to make sure building is successful - UI.success "✅ Build successful" + UI.message("🕗 Building page...") + sh("mkdocs build --clean --strict") # to make sure building is successful + UI.success("✅ Build successful") end private_lane :ensure_filenames do - UI.message "🕗 Making sure file names are all lowercase..." + UI.message("🕗 Making sure file names are all lowercase...") Dir["docs/**/*.md"].each do |current_file| if current_file =~ /[A-Z]/ UI.user_error!(".md files must not be upper case, use dash instead (file '#{current_file}')") end end - UI.success "✅ All doc files are properly named to not be camel case" + UI.success("✅ All doc files are properly named to not be camel case") end private_lane :ensure_redirects do - UI.message "🕗 Making sure the forwards are set up properly..." + UI.message("🕗 Making sure the forwards are set up properly...") require_relative "scripts/ci/available_redirects" docs_redirects.each do |from, to| @@ -50,12 +50,11 @@ private_lane :ensure_redirects do UI.user_error!("Couldn't find file that is being redirected to '#{full_to_path}' from '#{from}'") end end - UI.success "✅ All forwards are cool" + UI.success("✅ All forwards are cool") end - private_lane :ensure_image_assets do - UI.message "🕗 Checking docs for dead image references..." + UI.message("🕗 Checking docs for dead image references...") errors = [] Dir["docs/**/*.md"].each do |current_file| content = File.read(current_file) @@ -82,13 +81,13 @@ private_lane :ensure_image_assets do end errors.each { |e| UI.error(e) } UI.user_error!("At least one image file was not found, check out error above") unless errors.empty? - UI.success "✅ All image files were found" + UI.success("✅ All image files were found") end # This will verify that no non-md files are inside each of the sub-directories # All images must be stored in the correct sub-dir private_lane :ensure_assets_location do - UI.message "🕗 Ensuring asset locations..." + UI.message("🕗 Ensuring asset locations...") error = false Dir["docs/**/*"].each do |path| next if File.directory?(path) @@ -99,7 +98,7 @@ private_lane :ensure_assets_location do UI.error("File '#{path}' shouldn't be in this directory, only `.md` files are allowed") end UI.user_error!("Found some files in a wrong directory") if error - UI.success "✅ All assets are in the correct subfolder" + UI.success("✅ All assets are in the correct subfolder") end # This ensures the correct formatting for the fastlane tools @@ -124,7 +123,7 @@ end # Run the code samples, yo private_lane :ensure_code_samples do - UI.message "🕗 Verifying all code samples work with the latest fastlane release" + UI.message("🕗 Verifying all code samples work with the latest fastlane release") all_content = "" files = Dir["docs/actions/*.md"] files.each do |tool_sub_page_path| diff --git a/fastlane/actions/test_sample_code.rb b/fastlane/actions/test_sample_code.rb index c09e1b73cc..eb5eb305f0 100644 --- a/fastlane/actions/test_sample_code.rb +++ b/fastlane/actions/test_sample_code.rb @@ -93,7 +93,7 @@ def self.blacklist def self.available_options [ FastlaneCore::ConfigItem.new(key: :path), - FastlaneCore::ConfigItem.new(key: :content), + FastlaneCore::ConfigItem.new(key: :content) ] end diff --git a/scripts/ci/generate_redirects.rb b/scripts/ci/generate_redirects.rb index fc4e495b90..363617893d 100644 --- a/scripts/ci/generate_redirects.rb +++ b/scripts/ci/generate_redirects.rb @@ -1,8 +1,8 @@ require_relative "available_redirects" require 'fileutils' -puts "Generating redirects for the following URLs:" -puts docs_redirects +puts("Generating redirects for the following URLs:") +puts(docs_redirects) docs_redirects.each do |from, to| FileUtils.mkdir_p(from) @@ -12,5 +12,5 @@ content = "" File.write(full_path, content) - puts "Successfully created file at path '#{full_path}' to point to '#{to}'" + puts("Successfully created file at path '#{full_path}' to point to '#{to}'") end