Merge pull request #9690 from ggarnier/master

.rubocop.yml

@@ -932,7 +932,7 @@ Lint/UselessAccessModifier:
 Lint/UselessAssignment:
   Description: 'Checks for useless assignment to a local variable.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#underscore-unused-vars'
-  Enabled: false
+  Enabled: true
 
 Lint/UselessComparison:
   Description: 'Checks for comparison of something with itself.'

app/controllers/ci/lints_controller.rb

@@ -18,7 +18,7 @@ module Ci
     rescue Ci::GitlabCiYamlProcessor::ValidationError => e
       @error = e.message
       @status = false
-    rescue Exception => e
+    rescue Exception
       @error = "Undefined error"
       @status = false
     end

app/controllers/omniauth_callbacks_controller.rb

@@ -71,7 +71,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
         redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return
       end
     end
-  rescue Gitlab::OAuth::SignupDisabledError => e
+  rescue Gitlab::OAuth::SignupDisabledError
     label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
     message = "Signing in using your #{label} account without a pre-existing GitLab account is not allowed."
 
@@ -80,7 +80,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     end
 
     flash[:notice] = message
-    
+
     redirect_to new_user_session_path
   end
 

app/controllers/projects/wikis_controller.rb

@@ -98,7 +98,7 @@ class Projects::WikisController < Projects::ApplicationController
 
     # Call #wiki to make sure the Wiki Repo is initialized
     @project_wiki.wiki
-  rescue ProjectWiki::CouldNotCreateWikiError => ex
+  rescue ProjectWiki::CouldNotCreateWikiError
     flash[:notice] = "Could not create Wiki Repository at this time. Please try again later."
     redirect_to project_path(@project)
     return false

app/finders/issuable_finder.rb

@@ -39,7 +39,7 @@ class IssuableFinder
     items = by_assignee(items)
     items = by_author(items)
     items = by_label(items)
-    items = sort(items)
+    sort(items)
   end
 
   def group

app/helpers/application_helper.rb

@@ -35,7 +35,7 @@ module ApplicationHelper
   def project_icon(project_id, options = {})
     project =
       if project_id.is_a?(Project)
-        project = project_id
+        project_id
       else
         Project.find_with_namespace(project_id)
       end

app/models/ci/build.rb

@@ -26,7 +26,7 @@
 module Ci
   class Build < ActiveRecord::Base
     extend Ci::Model
-    
+
     LAZY_ATTRIBUTES = ['trace']
 
     belongs_to :commit, class_name: 'Ci::Commit'
@@ -140,7 +140,7 @@ module Ci
 
     def trace_html
       html = Ci::Ansi2html::convert(trace) if trace.present?
-      html ||= ''
+      html || ''
     end
 
     def started?
@@ -211,7 +211,7 @@ module Ci
         if coverage.present?
           coverage.to_f
         end
-      rescue => ex
+      rescue
         # if bad regex or something goes wrong we dont want to interrupt transition
         # so we just silentrly ignore error for now
       end

app/models/ci/project.rb

@@ -184,7 +184,7 @@ module Ci
 
         # If service is available but missing in db
         # we should create an instance. Ex `create_gitlab_ci_service`
-        service = self.send :"create_#{service_name}_service" if service.nil?
+        self.send :"create_#{service_name}_service" if service.nil?
       end
     end
 

app/models/merge_request_diff.rb

@@ -144,12 +144,10 @@ class MergeRequestDiff < ActiveRecord::Base
   # Collect array of Git::Diff objects
   # between target and source branches
   def unmerged_diffs
-    diffs = compare_result.diffs
-    diffs ||= []
-    diffs
-  rescue Gitlab::Git::Diff::TimeoutError => ex
+    compare_result.diffs || []
+  rescue Gitlab::Git::Diff::TimeoutError
     self.state = :timeout
-    diffs = []
+    []
   end
 
   def repository

app/models/project.rb

@@ -413,7 +413,7 @@ class Project < ActiveRecord::Base
 
         if template.nil?
           # If no template, we should create an instance. Ex `create_gitlab_ci_service`
-          service = self.send :"create_#{service_name}_service"
+          self.send :"create_#{service_name}_service"
         else
           Service.create_from_template(self.id, template)
         end
@@ -738,7 +738,7 @@ class Project < ActiveRecord::Base
   def create_wiki
     ProjectWiki.new(self, self.owner).wiki
     true
-  rescue ProjectWiki::CouldNotCreateWikiError => ex
+  rescue ProjectWiki::CouldNotCreateWikiError
     errors.add(:base, 'Failed create wiki')
     false
   end

app/models/project_services/hipchat_service.rb

@@ -85,17 +85,16 @@ class HipchatService < Service
   def create_message(data)
     object_kind = data[:object_kind]
 
-    message = \
-      case object_kind
-      when "push", "tag_push"
-        create_push_message(data)
-      when "issue"
-        create_issue_message(data) unless is_update?(data)
-      when "merge_request"
-        create_merge_request_message(data) unless is_update?(data)
-      when "note"
-        create_note_message(data)
-      end
+    case object_kind
+    when "push", "tag_push"
+      create_push_message(data)
+    when "issue"
+      create_issue_message(data) unless is_update?(data)
+    when "merge_request"
+      create_merge_request_message(data) unless is_update?(data)
+    when "note"
+      create_note_message(data)
+    end
   end
 
   def create_push_message(push)
@@ -167,8 +166,6 @@ class HipchatService < Service
     obj_attr = data[:object_attributes]
     obj_attr = HashWithIndifferentAccess.new(obj_attr)
     merge_request_id = obj_attr[:iid]
-    source_branch = obj_attr[:source_branch]
-    target_branch = obj_attr[:target_branch]
     state = obj_attr[:state]
     description = obj_attr[:description]
     title = obj_attr[:title]
@@ -194,8 +191,6 @@ class HipchatService < Service
     data = HashWithIndifferentAccess.new(data)
     user_name = data[:user][:name]
 
-    repo_attr = HashWithIndifferentAccess.new(data[:repository])
-
     obj_attr = HashWithIndifferentAccess.new(data[:object_attributes])
     note = obj_attr[:note]
     note_url = obj_attr[:url]

app/models/repository.rb

@@ -549,7 +549,7 @@ class Repository
 
       # Run GitLab post receive hook
       post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo)
-      status = post_receive_hook.trigger(gl_id, oldrev, newrev, ref)
+      post_receive_hook.trigger(gl_id, oldrev, newrev, ref)
     else
       # Remove tmp ref and return error to user
       rugged.references.delete(tmp_ref)

app/models/user.rb

@@ -705,13 +705,7 @@ class User < ActiveRecord::Base
   end
 
   def manageable_namespaces
-    @manageable_namespaces ||=
-      begin
-        namespaces = []
-        namespaces << namespace
-        namespaces += owned_groups
-        namespaces += masters_groups
-      end
+    @manageable_namespaces ||= [namespace] + owned_groups + masters_groups
   end
 
   def namespaces

app/services/files/base_service.rb

@@ -21,7 +21,7 @@ module Files
         create_target_branch
       end
 
-      if sha = commit
+      if commit
         success
       else
         error("Something went wrong. Your changes were not committed")

app/services/projects/create_service.rb

@@ -62,7 +62,7 @@ module Projects
       after_create_actions if @project.persisted?
 
       @project
-    rescue => ex
+    rescue
       @project.errors.add(:base, "Can't save project. Please try again later")
       @project
     end

features/steps/groups.rb

@@ -282,9 +282,9 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
     milestone1_project2 = create :milestone,
                             title: "Version 7.2",
                             project: project2
-    milestone1_project3 = create :milestone,
-                            title: "Version 7.2",
-                            project: @project3
+    create :milestone,
+      title: "Version 7.2",
+      project: @project3
     milestone2_project1 = create :milestone,
                             title: "GL-113",
                             project: @project1
@@ -301,28 +301,28 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
                assignee: current_user,
                author: current_user,
                milestone: milestone2_project1
-    issue2 = create :issue,
-               project: project2,
-               assignee: current_user,
-               author: current_user,
-               milestone: milestone1_project2
-    issue3 = create :issue,
-               project: @project3,
-               assignee: current_user,
-               author: current_user,
-               milestone: milestone1_project1
-    mr1 = create :merge_request,
-            source_project: @project1,
-            target_project: @project1,
-            assignee: current_user,
-            author: current_user,
-            milestone: milestone2_project1
-    mr2 = create :merge_request,
-            source_project: project2,
-            target_project: project2,
-            assignee: current_user,
-            author: current_user,
-            milestone: milestone2_project2
+    create :issue,
+      project: project2,
+      assignee: current_user,
+      author: current_user,
+      milestone: milestone1_project2
+    create :issue,
+      project: @project3,
+      assignee: current_user,
+      author: current_user,
+      milestone: milestone1_project1
+    create :merge_request,
+      source_project: @project1,
+      target_project: @project1,
+      assignee: current_user,
+      author: current_user,
+      milestone: milestone2_project1
+    create :merge_request,
+      source_project: project2,
+      target_project: project2,
+      assignee: current_user,
+      author: current_user,
+      milestone: milestone2_project2
     @mr3 = create :merge_request,
             source_project: @project3,
             target_project: @project3,

features/steps/project/graph.rb

@@ -32,6 +32,6 @@ class Spinach::Features::ProjectGraph < Spinach::FeatureSteps
   end
 
   def project
-    project ||= Project.find_by(name: "Shop")
+    @project ||= Project.find_by(name: "Shop")
   end
 end

features/steps/project/issues/issues.rb

@@ -223,11 +223,11 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
   end
 
   step 'project \'Shop\' has issue \'Bugfix1\' with description: \'Description for issue1\'' do
-    issue = create(:issue, title: 'Bugfix1', description: 'Description for issue1', project: project)
+    create(:issue, title: 'Bugfix1', description: 'Description for issue1', project: project)
   end
 
   step 'project \'Shop\' has issue \'Feature1\' with description: \'Feature submitted for issue1\'' do
-    issue = create(:issue, title: 'Feature1', description: 'Feature submitted for issue1', project: project)
+    create(:issue, title: 'Feature1', description: 'Feature submitted for issue1', project: project)
   end
 
   step 'I fill in issue search with \'Description for issue1\'' do

features/steps/project/redirects.rb

@@ -39,7 +39,6 @@ class Spinach::Features::ProjectRedirects < Spinach::FeatureSteps
 
   step 'Authenticate' do
     admin = create(:admin)
-    project = Project.find_by(name: 'Community')
     fill_in "user_login", with: admin.email
     fill_in "user_password", with: admin.password
     click_button "Sign in"
@@ -54,7 +53,6 @@ class Spinach::Features::ProjectRedirects < Spinach::FeatureSteps
 
   step 'I get redirected to signin page where I sign in' do
     admin = create(:admin)
-    project = Project.find_by(name: 'Enterprise')
     fill_in "user_login", with: admin.email
     fill_in "user_password", with: admin.password
     click_button "Sign in"

features/steps/shared/group.rb

@@ -37,7 +37,7 @@ module SharedGroup
     group = Group.find_by(name: groupname) || create(:group, name: groupname)
     group.add_user(user, role)
     project ||= create(:project, namespace: group, path: "project#{@project_count}")
-    event   ||= create(:closed_issue_event, project: project)
+    create(:closed_issue_event, project: project)
     project.team << [user, :master]
     @project_count += 1
   end

lib/api/helpers.rb

@@ -63,11 +63,11 @@ module API
           user_project.build_missing_services
 
           service_method = "#{underscored_service}_service"
-          
+
           send_service(service_method)
         end
       end
-   
+
       @project_service || not_found!("Service")
     end
 
@@ -149,7 +149,6 @@ module API
     end
 
     def attributes_for_keys(keys, custom_params = nil)
-      params_hash = custom_params || params
       attrs = {}
       keys.each do |key|
         if params[key].present? or (params.has_key?(key) and params[key] == false)

lib/event_filter.rb

@@ -47,7 +47,7 @@ class EventFilter
 
     actions << Event::COMMENTED if filter.include? 'comments'
 
-    events = events.where(action: actions)
+    events.where(action: actions)
   end
 
   def options(key)

lib/gitlab/contributions_calendar.rb

@@ -12,7 +12,6 @@ module Gitlab
 
       @timestamps = {}
       date_from = 1.year.ago
-      date_to = Date.today
 
       events = Event.reorder(nil).contributions.where(author_id: user.id).
         where("created_at > ?", date_from).where(project_id: projects).

lib/gitlab/diff/parser.rb

@@ -14,8 +14,6 @@ module Gitlab
         lines_arr = ::Gitlab::InlineDiff.processing lines
 
         lines_arr.each do |line|
-          raw_line = line.dup
-
           next if filename?(line)
 
           full_line = html_escape(line.gsub(/\n/, ''))

lib/gitlab/fogbugz_import/project_creator.rb

@@ -23,7 +23,7 @@ module Gitlab
           import_url: Project::UNKNOWN_IMPORT_URL
         ).execute
 
-        import_data = project.create_import_data(
+        project.create_import_data(
           data: {
             'repo' => repo.raw_data,
             'user_map' => user_map,

lib/gitlab/google_code_import/project_creator.rb

@@ -23,7 +23,7 @@ module Gitlab
           import_url: repo.import_url
         ).execute
 
-        import_data = project.create_import_data(
+        project.create_import_data(
           data: {
             "repo"      => repo.raw_data,
             "user_map"  => user_map

spec/features/ci/admin/builds_spec.rb

@@ -21,10 +21,10 @@ describe "Admin Builds" do
 
   describe "Tabs" do
     it "shows all builds" do
-      build = FactoryGirl.create :ci_build, commit: commit, status: "pending"
-      build1 = FactoryGirl.create :ci_build, commit: commit, status: "running"
-      build2 = FactoryGirl.create :ci_build, commit: commit, status: "success"
-      build3 = FactoryGirl.create :ci_build, commit: commit, status: "failed"
+      FactoryGirl.create :ci_build, commit: commit, status: "pending"
+      FactoryGirl.create :ci_build, commit: commit, status: "running"
+      FactoryGirl.create :ci_build, commit: commit, status: "success"
+      FactoryGirl.create :ci_build, commit: commit, status: "failed"
 
       visit ci_admin_builds_path
 

spec/lib/gitlab/backend/grack_auth_spec.rb

@@ -151,14 +151,14 @@ describe Grack::Auth do
                 end
 
                 it "repeated attempts followed by successful attempt" do
-                  for n in 0..maxretry do
+                  maxretry.times.each do
                     expect(attempt_login(false)).to eq(401)
                   end
 
                   expect(attempt_login(true)).to eq(200)
                   expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey
 
-                  for n in 0..maxretry do
+                  maxretry.times.each do
                     expect(attempt_login(false)).to eq(401)
                   end
                 end

spec/lib/gitlab/o_auth/user_spec.rb

@@ -19,10 +19,6 @@ describe Gitlab::OAuth::User do
     let!(:existing_user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'my-provider') }
 
     it "finds an existing user based on uid and provider (facebook)" do
-      # FIXME (rspeicher): It's unlikely that this test is actually doing anything
-      # `auth` is never used and removing it entirely doesn't break the test, so
-      # what's it doing?
-      auth = double(info: double(name: 'John'), uid: 'my-uid', provider: 'my-provider')
       expect( oauth_user.persisted? ).to be_truthy
     end
 

spec/models/broadcast_message_spec.rb

@@ -27,12 +27,12 @@ describe BroadcastMessage do
     end
 
     it "should return nil if time not come" do
-      broadcast_message = create(:broadcast_message, starts_at: Time.now.tomorrow, ends_at: Time.now + 2.days)
+      create(:broadcast_message, starts_at: Time.now.tomorrow, ends_at: Time.now + 2.days)
       expect(BroadcastMessage.current).to be_nil
     end
 
     it "should return nil if time has passed" do
-      broadcast_message = create(:broadcast_message, starts_at: Time.now - 2.days, ends_at: Time.now.yesterday)
+      create(:broadcast_message, starts_at: Time.now - 2.days, ends_at: Time.now.yesterday)
       expect(BroadcastMessage.current).to be_nil
     end
   end

spec/models/ci/commit_spec.rb

@@ -228,13 +228,13 @@ describe Ci::Commit do
 
     it "returns finished_at of latest build" do
       build = FactoryGirl.create :ci_build, commit: commit, finished_at: Time.now - 60
-      build1 = FactoryGirl.create :ci_build, commit: commit, finished_at: Time.now - 120
+      FactoryGirl.create :ci_build, commit: commit, finished_at: Time.now - 120
 
       expect(commit.finished_at.to_i).to eq(build.finished_at.to_i)
     end
 
     it "returns nil if there is no finished build" do
-      build = FactoryGirl.create :ci_not_started_build, commit: commit
+      FactoryGirl.create :ci_not_started_build, commit: commit
 
       expect(commit.finished_at).to be_nil
     end

spec/models/hooks/project_hook_spec.rb

@@ -22,7 +22,7 @@ describe ProjectHook do
   describe '.push_hooks' do
     it 'should return hooks for push events only' do
       hook = create(:project_hook, push_events: true)
-      hook2 = create(:project_hook, push_events: false)
+      create(:project_hook, push_events: false)
       expect(ProjectHook.push_hooks).to eq([hook])
     end
   end
@@ -30,7 +30,7 @@ describe ProjectHook do
   describe '.tag_push_hooks' do
     it 'should return hooks for tag push events only' do
       hook = create(:project_hook, tag_push_events: true)
-      hook2 = create(:project_hook, tag_push_events: false)
+      create(:project_hook, tag_push_events: false)
       expect(ProjectHook.tag_push_hooks).to eq([hook])
     end
   end

spec/models/hooks/service_hook_spec.rb

@@ -39,8 +39,6 @@ describe ServiceHook do
     end
 
     it "POSTs the data as JSON" do
-      json = @data.to_json
-
       @service_hook.execute(@data)
       expect(WebMock).to have_requested(:post, @service_hook.url).with(
         headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook' }

spec/models/hooks/web_hook_spec.rb

@@ -60,8 +60,6 @@ describe ProjectHook do
     end
 
     it "POSTs the data as JSON" do
-      json = @data.to_json
-
       @project_hook.execute(@data, 'push_hooks')
       expect(WebMock).to have_requested(:post, @project_hook.url).with(
         headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook' }

spec/models/milestone_spec.rb

@@ -111,8 +111,8 @@ describe Milestone do
 
   describe :is_empty? do
     before do
-      issue = create :closed_issue, milestone: milestone
-      merge_request = create :merge_request, milestone: milestone
+      create :closed_issue, milestone: milestone
+      create :merge_request, milestone: milestone
     end
 
     it 'Should return total count of issues and merge requests assigned to milestone' do
@@ -125,7 +125,7 @@ describe Milestone do
       milestone = create :milestone
       create :closed_issue, milestone: milestone
 
-      issue = create :issue
+      create :issue
     end
 
     it 'should be true if milestone active and all nested issues closed' do

spec/models/project_services/gitlab_ci_service_spec.rb

@@ -49,7 +49,6 @@ describe GitlabCiService do
       let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
 
       it "calls ci_yaml_file" do
-        service_hook = double
         expect(@service).to receive(:ci_yaml_file).with(push_sample_data[:checkout_sha])
 
         @service.execute(push_sample_data)

spec/models/project_services/hipchat_service_spec.rb

@@ -87,7 +87,7 @@ describe HipchatService do
       it "should create a push message" do
         message = hipchat.send(:create_push_message, push_sample_data)
 
-        obj_attr = push_sample_data[:object_attributes]
+        push_sample_data[:object_attributes]
         branch = push_sample_data[:ref].gsub('refs/heads/', '')
         expect(message).to include("#{user.name} pushed to branch " \
             "<a href=\"#{project.web_url}/commits/#{branch}\">#{branch}</a> of " \
@@ -107,7 +107,7 @@ describe HipchatService do
       it "should create a tag push message" do
         message = hipchat.send(:create_push_message, push_sample_data)
 
-        obj_attr = push_sample_data[:object_attributes]
+        push_sample_data[:object_attributes]
         expect(message).to eq("#{user.name} pushed new tag " \
             "<a href=\"#{project.web_url}/commits/test\">test</a> to " \
             "<a href=\"#{project.web_url}\">#{project_name}</a>\n")

spec/models/project_spec.rb

@@ -140,7 +140,7 @@ describe Project do
 
     describe 'last_activity_date' do
       it 'returns the creation date of the project\'s last event if present' do
-        last_activity_event = create(:event, project: project)
+        create(:event, project: project)
         expect(project.last_activity_at.to_i).to eq(last_event.created_at.to_i)
       end
 

spec/models/project_wiki_spec.rb

@@ -231,7 +231,7 @@ describe ProjectWiki do
   end
 
   def commit_details
-    commit = { name: user.name, email: user.email, message: "test commit" }
+    { name: user.name, email: user.email, message: "test commit" }
   end
 
   def create_page(name, content)

spec/models/wiki_page_spec.rb

@@ -196,7 +196,7 @@ describe WikiPage do
   end
 
   def commit_details
-    commit = { name: user.name, email: user.email, message: "test commit" }
+    { name: user.name, email: user.email, message: "test commit" }
   end
 
   def create_page(name, content)

spec/tasks/gitlab/backup_rake_spec.rb

@@ -159,7 +159,7 @@ describe 'gitlab:app namespace rake task' do
     end
 
     it "does not contain skipped item" do
-      tar_contents, exit_status = Gitlab::Popen.popen(
+      tar_contents, _exit_status = Gitlab::Popen.popen(
         %W{tar -tvf #{@backup_tar} db uploads repositories builds}
       )