Commit 0fb0f060 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'rubocop/enable-access-modifiers-cops' into 'master'

Rubocop - enable cops access modifiers

This is sync with https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5014

See merge request !603
parents 56b36c91 60e41187
......@@ -510,6 +510,15 @@ Metrics/PerceivedComplexity:
#################### Lint ################################
# Checks for useless access modifiers.
Lint/UselessAccessModifier:
Enabled: true
# Checks for attempts to use `private` or `protected` to set the visibility
# of a class method, which does not work.
Lint/IneffectiveAccessModifier:
Enabled: false
# Checks for ambiguous operators in the first argument of a method invocation
# without parentheses.
Lint/AmbiguousOperator:
......
......@@ -19,10 +19,6 @@ Lint/AssignmentInCondition:
Lint/HandleExceptions:
Enabled: false
# Offense count: 21
Lint/IneffectiveAccessModifier:
Enabled: false
# Offense count: 2
Lint/Loop:
Enabled: false
......@@ -48,10 +44,6 @@ Lint/UnusedBlockArgument:
Lint/UnusedMethodArgument:
Enabled: false
# Offense count: 11
Lint/UselessAccessModifier:
Enabled: false
# Offense count: 12
# Cop supports --auto-correct.
Performance/PushSplat:
......
......@@ -82,8 +82,6 @@ class Import::BitbucketController < Import::BaseController
go_to_bitbucket_for_permissions
end
private
def access_params
{
bitbucket_access_token: session[:bitbucket_access_token],
......
......@@ -61,8 +61,6 @@ class Import::GitlabController < Import::BaseController
go_to_gitlab_for_permissions
end
private
def access_params
{ gitlab_access_token: session[:gitlab_access_token] }
end
......
......@@ -144,8 +144,6 @@ module DiffHelper
toggle_whitespace_link(url, options)
end
private
def hide_whitespace?
params[:w] == '1'
end
......
module TokenAuthenticatable
extend ActiveSupport::Concern
private
def write_new_token(token_field)
new_token = generate_token(token_field)
write_attribute(token_field, new_token)
end
def generate_token(token_field)
loop do
token = Devise.friendly_token
break token unless self.class.unscoped.find_by(token_field => token)
end
end
class_methods do
def authentication_token_fields
@token_fields || []
end
private
private # rubocop:disable Lint/UselessAccessModifier
def add_authentication_token_field(token_field)
@token_fields = [] unless @token_fields
......@@ -32,18 +46,4 @@ module TokenAuthenticatable
end
end
end
private
def write_new_token(token_field)
new_token = generate_token(token_field)
write_attribute(token_field, new_token)
end
def generate_token(token_field)
loop do
token = Devise.friendly_token
break token unless self.class.unscoped.find_by(token_field => token)
end
end
end
......@@ -28,6 +28,10 @@ module Auth
token.encoded
end
def self.token_expire_at
Time.now + current_application_settings.container_registry_token_expire_delay.minutes
end
private
def authorized_token(*accesses)
......@@ -35,7 +39,7 @@ module Auth
token.issuer = registry.issuer
token.audience = params[:service]
token.subject = current_user.try(:username)
token.expire_time = ContainerRegistryAuthenticationService.token_expire_at
token.expire_time = self.class.token_expire_at
token[:access] = accesses.compact
token
end
......@@ -81,9 +85,5 @@ module Auth
def registry
Gitlab.config.registry
end
def self.token_expire_at
Time.now + current_application_settings.container_registry_token_expire_delay.minutes
end
end
end
......@@ -2,7 +2,9 @@
#
# Used for creating system notes (e.g., when a user references a merge request
# from an issue, an issue's assignee changes, an issue is closed, etc.)
class SystemNoteService
module SystemNoteService
extend self
# Called when commits are added to a Merge Request
#
# noteable - Noteable object
......@@ -15,7 +17,7 @@ class SystemNoteService
# See new_commit_summary and existing_commit_summary.
#
# Returns the created Note object
def self.add_commits(noteable, project, author, new_commits, existing_commits = [], oldrev = nil)
def add_commits(noteable, project, author, new_commits, existing_commits = [], oldrev = nil)
total_count = new_commits.length + existing_commits.length
commits_text = "#{total_count} commit".pluralize(total_count)
......@@ -40,7 +42,7 @@ class SystemNoteService
# "Reassigned to @rspeicher"
#
# Returns the created Note object
def self.change_assignee(noteable, project, author, assignee)
def change_assignee(noteable, project, author, assignee)
body = assignee.nil? ? 'Assignee removed' : "Reassigned to #{assignee.to_reference}"
create_note(noteable: noteable, project: project, author: author, note: body)
......@@ -63,7 +65,7 @@ class SystemNoteService
# "Removed ~5 label"
#
# Returns the created Note object
def self.change_label(noteable, project, author, added_labels, removed_labels)
def change_label(noteable, project, author, added_labels, removed_labels)
labels_count = added_labels.count + removed_labels.count
references = ->(label) { label.to_reference(format: :id) }
......@@ -101,7 +103,7 @@ class SystemNoteService
# "Miletone changed to 7.11"
#
# Returns the created Note object
def self.change_milestone(noteable, project, author, milestone)
def change_milestone(noteable, project, author, milestone)
body = 'Milestone '
body += milestone.nil? ? 'removed' : "changed to #{milestone.to_reference(project)}"
......@@ -123,7 +125,7 @@ class SystemNoteService
# "Status changed to closed by bc17db76"
#
# Returns the created Note object
def self.change_status(noteable, project, author, status, source)
def change_status(noteable, project, author, status, source)
body = "Status changed to #{status}"
body << " by #{source.gfm_reference(project)}" if source
......@@ -131,26 +133,26 @@ class SystemNoteService
end
# Called when 'merge when build succeeds' is executed
def self.merge_when_build_succeeds(noteable, project, author, last_commit)
def merge_when_build_succeeds(noteable, project, author, last_commit)
body = "Enabled an automatic merge when the build for #{last_commit.to_reference(project)} succeeds"
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when 'merge when build succeeds' is canceled
def self.cancel_merge_when_build_succeeds(noteable, project, author)
def cancel_merge_when_build_succeeds(noteable, project, author)
body = 'Canceled the automatic merge'
create_note(noteable: noteable, project: project, author: author, note: body)
end
def self.remove_merge_request_wip(noteable, project, author)
def remove_merge_request_wip(noteable, project, author)
body = 'Unmarked this merge request as a Work In Progress'
create_note(noteable: noteable, project: project, author: author, note: body)
end
def self.add_merge_request_wip(noteable, project, author)
def add_merge_request_wip(noteable, project, author)
body = 'Marked this merge request as a **Work In Progress**'
create_note(noteable: noteable, project: project, author: author, note: body)
......@@ -168,7 +170,7 @@ class SystemNoteService
# "Title changed from **Old** to **New**"
#
# Returns the created Note object
def self.change_title(noteable, project, author, old_title)
def change_title(noteable, project, author, old_title)
new_title = noteable.title.dup
old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_title, new_title).inline_diffs
......@@ -191,7 +193,7 @@ class SystemNoteService
# "Made the issue confidential"
#
# Returns the created Note object
def self.change_issue_confidentiality(issue, project, author)
def change_issue_confidentiality(issue, project, author)
body = issue.confidential ? 'Made the issue confidential' : 'Made the issue visible'
create_note(noteable: issue, project: project, author: author, note: body)
end
......@@ -210,7 +212,7 @@ class SystemNoteService
# "Target branch changed from `Old` to `New`"
#
# Returns the created Note object
def self.change_branch(noteable, project, author, branch_type, old_branch, new_branch)
def change_branch(noteable, project, author, branch_type, old_branch, new_branch)
body = "#{branch_type} branch changed from `#{old_branch}` to `#{new_branch}`".capitalize
create_note(noteable: noteable, project: project, author: author, note: body)
end
......@@ -229,7 +231,7 @@ class SystemNoteService
# "Restored target branch `feature`"
#
# Returns the created Note object
def self.change_branch_presence(noteable, project, author, branch_type, branch, presence)
def change_branch_presence(noteable, project, author, branch_type, branch, presence)
verb =
if presence == :add
'restored'
......@@ -245,7 +247,7 @@ class SystemNoteService
# Example note text:
#
# "Started branch `201-issue-branch-button`"
def self.new_issue_branch(issue, project, author, branch)
def new_issue_branch(issue, project, author, branch)
h = Gitlab::Routing.url_helpers
link = h.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch)
......@@ -270,7 +272,7 @@ class SystemNoteService
# See cross_reference_note_content.
#
# Returns the created Note object
def self.cross_reference(noteable, mentioner, author)
def cross_reference(noteable, mentioner, author)
return if cross_reference_disallowed?(noteable, mentioner)
gfm_reference = mentioner.gfm_reference(noteable.project)
......@@ -294,7 +296,7 @@ class SystemNoteService
end
end
def self.cross_reference?(note_text)
def cross_reference?(note_text)
note_text.start_with?(cross_reference_note_prefix)
end
......@@ -308,7 +310,7 @@ class SystemNoteService
# mentioner - Mentionable object
#
# Returns Boolean
def self.cross_reference_disallowed?(noteable, mentioner)
def cross_reference_disallowed?(noteable, mentioner)
return true if noteable.is_a?(ExternalIssue) && !noteable.project.jira_tracker_active?
return false unless mentioner.is_a?(MergeRequest)
return false unless noteable.is_a?(Commit)
......@@ -328,7 +330,7 @@ class SystemNoteService
#
# Returns Boolean
def self.cross_reference_exists?(noteable, mentioner)
def cross_reference_exists?(noteable, mentioner)
# Initial scope should be system notes of this noteable type
notes = Note.system.where(noteable_type: noteable.class)
......@@ -342,6 +344,57 @@ class SystemNoteService
notes_for_mentioner(mentioner, noteable, notes).count > 0
end
# Build an Array of lines detailing each commit added in a merge request
#
# new_commits - Array of new Commit objects
#
# Returns an Array of Strings
def new_commit_summary(new_commits)
new_commits.collect do |commit|
"* #{commit.short_id} - #{escape_html(commit.title)}"
end
end
# Called when the status of a Task has changed
#
# noteable - Noteable object.
# project - Project owning noteable
# author - User performing the change
# new_task - TaskList::Item object.
#
# Example Note text:
#
# "Soandso marked the task Whatever as completed."
#
# Returns the created Note object
def change_task_status(noteable, project, author, new_task)
status_label = new_task.complete? ? Taskable::COMPLETED : Taskable::INCOMPLETE
body = "Marked the task **#{new_task.source}** as #{status_label}"
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when noteable has been moved to another project
#
# direction - symbol, :to or :from
# noteable - Noteable object
# noteable_ref - Referenced noteable
# author - User performing the move
#
# Example Note text:
#
# "Moved to some_namespace/project_new#11"
#
# Returns the created Note object
def noteable_moved(noteable, project, noteable_ref, author, direction:)
unless [:to, :from].include?(direction)
raise ArgumentError, "Invalid direction `#{direction}`"
end
cross_reference = noteable_ref.to_reference(project)
body = "Moved #{direction} #{cross_reference}"
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when the merge request is approved by user
#
# noteable - Noteable object
......@@ -352,7 +405,7 @@ class SystemNoteService
# "Approved this merge request"
#
# Returns the created Note object
def self.approve_mr(noteable, user)
def approve_mr(noteable, user)
body = "Approved this merge request"
create_note(noteable: noteable, project: noteable.project, author: user, note: body)
......@@ -360,7 +413,7 @@ class SystemNoteService
private
def self.notes_for_mentioner(mentioner, noteable, notes)
def notes_for_mentioner(mentioner, noteable, notes)
if mentioner.is_a?(Commit)
notes.where('note LIKE ?', "#{cross_reference_note_prefix}%#{mentioner.to_reference(nil)}")
else
......@@ -369,29 +422,18 @@ class SystemNoteService
end
end
def self.create_note(args = {})
def create_note(args = {})
Note.create(args.merge(system: true))
end
def self.cross_reference_note_prefix
def cross_reference_note_prefix
'mentioned in '
end
def self.cross_reference_note_content(gfm_reference)
def cross_reference_note_content(gfm_reference)
"#{cross_reference_note_prefix}#{gfm_reference}"
end
# Build an Array of lines detailing each commit added in a merge request
#
# new_commits - Array of new Commit objects
#
# Returns an Array of Strings
def self.new_commit_summary(new_commits)
new_commits.collect do |commit|
"* #{commit.short_id} - #{escape_html(commit.title)}"
end
end
# Build a single line summarizing existing commits being added in a merge
# request
#
......@@ -408,7 +450,7 @@ class SystemNoteService
# "* ea0f8418 - 1 commit from branch `feature`"
#
# Returns a newline-terminated String
def self.existing_commit_summary(noteable, existing_commits, oldrev = nil)
def existing_commit_summary(noteable, existing_commits, oldrev = nil)
return '' if existing_commits.empty?
count = existing_commits.size
......@@ -431,47 +473,7 @@ class SystemNoteService
"* #{commit_ids} - #{commits_text} from branch `#{branch}`\n"
end
# Called when the status of a Task has changed
#
# noteable - Noteable object.
# project - Project owning noteable
# author - User performing the change
# new_task - TaskList::Item object.
#
# Example Note text:
#
# "Soandso marked the task Whatever as completed."
#
# Returns the created Note object
def self.change_task_status(noteable, project, author, new_task)
status_label = new_task.complete? ? Taskable::COMPLETED : Taskable::INCOMPLETE
body = "Marked the task **#{new_task.source}** as #{status_label}"
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when noteable has been moved to another project
#
# direction - symbol, :to or :from
# noteable - Noteable object
# noteable_ref - Referenced noteable
# author - User performing the move
#
# Example Note text:
#
# "Moved to some_namespace/project_new#11"
#
# Returns the created Note object
def self.noteable_moved(noteable, project, noteable_ref, author, direction:)
unless [:to, :from].include?(direction)
raise ArgumentError, "Invalid direction `#{direction}`"
end
cross_reference = noteable_ref.to_reference(project)
body = "Moved #{direction} #{cross_reference}"
create_note(noteable: noteable, project: project, author: author, note: body)
end
def self.escape_html(text)
def escape_html(text)
Rack::Utils.escape_html(text)
end
end
......@@ -38,6 +38,11 @@ module Banzai
end
end
# Build a regexp that matches all valid :emoji: names.
def self.emoji_pattern
@emoji_pattern ||= /:(#{Gitlab::Emoji.emojis_names.map { |name| Regexp.escape(name) }.join('|')}):/
end
private
def emoji_url(name)
......@@ -59,11 +64,6 @@ module Banzai
ActionController::Base.helpers.url_to_image(image)
end
# Build a regexp that matches all valid :emoji: names.
def self.emoji_pattern
@emoji_pattern ||= /:(#{Gitlab::Emoji.emojis_names.map { |name| Regexp.escape(name) }.join('|')}):/
end
def emoji_pattern
self.class.emoji_pattern
end
......
......@@ -12,7 +12,12 @@ module Banzai
html
end
private
def self.renderer
@renderer ||= begin
renderer = Redcarpet::Render::HTML.new
Redcarpet::Markdown.new(renderer, redcarpet_options)
end
end
def self.redcarpet_options
# https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use
......@@ -28,12 +33,7 @@ module Banzai
}.freeze
end
def self.renderer
@renderer ||= begin
renderer = Redcarpet::Render::HTML.new
Redcarpet::Markdown.new(renderer, redcarpet_options)
end
end
private_class_method :redcarpet_options
end
end
end
module Banzai
module Renderer
extend self
# Convert a Markdown String into an HTML-safe String of HTML
#
# Note that while the returned HTML will have been sanitized of dangerous
......@@ -14,7 +16,7 @@ module Banzai
# context - Hash of context options passed to our HTML Pipeline
#
# Returns an HTML-safe String
def self.render(text, context = {})
def render(text, context = {})
cache_key = context.delete(:cache_key)
cache_key = full_cache_key(cache_key, context[:pipeline])
......@@ -52,7 +54,7 @@ module Banzai
# texts_and_contexts
# => [{ text: '### Hello',
# context: { cache_key: [note, :note] } }]
def self.cache_collection_render(texts_and_contexts)
def cache_collection_render(texts_and_contexts)
items_collection = texts_and_contexts.each_with_index do |item, index|
context = item[:context]
cache_key = full_cache_multi_key(context.delete(:cache_key), context[:pipeline])
......@@ -81,7 +83,7 @@ module Banzai
items_collection.map { |item| item[:rendered] }
end
def self.render_result(text, context = {})
def render_result(text, context = {})
text = Pipeline[:pre_process].to_html(text, context) if text
Pipeline[context[:pipeline]].call(text, context)
......@@ -100,7 +102,7 @@ module Banzai
# :user - User object
#
# Returns an HTML-safe String
def self.post_process(html, context)
def post_process(html, context)
context = Pipeline[context[:pipeline]].transform_context(context)
pipeline = Pipeline[:post_process]
......@@ -113,7 +115,7 @@ module Banzai
private
def self.cacheless_render(text, context = {})
def cacheless_render(text, context = {})
Gitlab::Metrics.measure(:banzai_cacheless_render) do
result = render_result(text, context)
......@@ -126,7 +128,7 @@ module Banzai
end
end
def self.full_cache_key(cache_key, pipeline_name)
def full_cache_key(cache_key, pipeline_name)
return unless cache_key
["banzai", *cache_key, pipeline_name || :full]
end
......@@ -134,7 +136,7 @@ module Banzai
# To map Rails.cache.read_multi results we need to know the Rails.cache.expanded_key.
# Other option will be to generate stringified keys on our side and don't delegate to Rails.cache.expanded_key
# method.
def self.full_cache_multi_key(cache_key, pipeline_name)
def full_cache_multi_key(cache_key, pipeline_name)
return unless cache_key
Rails.cache.send(:expanded_key, full_cache_key(cache_key, pipeline_name))
end
......
......@@ -36,7 +36,7 @@ module Gitlab
Hash[(@nodes || {}).map { |key, factory| [key, factory.dup] }]
end
private
private # rubocop:disable Lint/UselessAccessModifier
def node(symbol, entry_class, metadata)
factory = Node::Factory.new(entry_class)
......
......@@ -55,12 +55,12 @@ module Gitlab
end
end
private
def self.connection
ActiveRecord::Base.connection
end
private_class_method :connection
def self.database_version
row = connection.execute("SELECT VERSION()").first
......@@ -70,5 +70,7 @@ module Gitlab
row.first
end
end
private_class_method :database_version
end
end
......@@ -19,24 +19,6 @@ module Gitlab
attr_accessor :old_line, :new_line, :offset
def self.for_lines(lines)
changed_line_pairs = self.find_changed_line_pairs(lines)
inline_diffs = []
changed_line_pairs.each do |old_index, new_index|
old_line = lines[old_index]
new_line = lines[new_index]
old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs
inline_diffs[old_index] = old_diffs
inline_diffs[new_index] = new_diffs
end
inline_diffs
end
def initialize(old_line, new_line, offset: 0)
@old_line = old_line[offset..-1]
@new_line = new_line[offset..-1]
......@@ -63,10 +45,29 @@ module Gitlab
[old_diffs, new_diffs]
end
class << self
def for_lines(lines)
changed_line_pairs = find_changed_line_pairs(lines)
inline_diffs = []
changed_line_pairs.each do |old_index, new_index|
old_line = lines[old_index]
new_line = lines[new_index]
old_diffs, new_diffs = new(old_line, new_line, offset: 1).inline_diffs
inline_diffs[old_index] = old_diffs
inline_diffs[new_index] = new_diffs
end
inline_diffs
end
private
# Finds pairs of old/new line pairs that represent the same line that changed
def self.find_changed_line_pairs(lines)
def find_changed_line_pairs(lines)
# Prefixes of all diff lines, indicating their types
# For example: `" - + -+ ---+++ --+ -++"`
line_prefixes = lines.each_with_object("") { |line, s| s << line[0] }.gsub(/[^ +-]/, ' ')
......@@ -88,6 +89,9 @@ module Gitlab
changed_line_pairs
end
end
private
def longest_common_prefix(a, b)
max_length = [a.length, b.length].max
......
......@@ -211,8 +211,6 @@ module Gitlab
project.repository.tag_exists?(tag_name)
end
private
def deploy_key
actor if actor.is_a?(DeployKey)
end
......
......@@ -136,10 +136,10 @@ module Gitlab
end
end
private
def self.current_transaction
Transaction.current
end
private_class_method :current_transaction
end
end
......@@ -2,6 +2,8 @@ module Gitlab
# Module containing GitLab's application theme definitions and helper methods
# for accessing them.
module Themes
extend self
# Theme ID used when no `default_theme` configuration setting is provided.
APPLICATION_DEFAULT = 2
......@@ -22,7 +24,7 @@ module Gitlab
# classes that might be applied to the `body` element
#
# Returns a String
def self.body_classes
def body_classes
THEMES.collect(&:css_class).uniq.join(' ')
end
......@@ -33,26 +35,26 @@ module Gitlab
# id - Integer ID
#
# Returns a Theme
def self.by_id(id)
def by_id(id)
THEMES.detect { |t| t.id == id } || default
end
# Returns the number of defined Themes
def self.count
def count
THEMES.size
end
# Get the default Theme
#
# Returns a Theme
def self.default
def default
by_id(default_id)
end
# Iterate through each Theme
#
# Yields the Theme object
def self.each(&block)
def each(&block)
THEMES.each(&block)
end
......@@ -61,7 +63,7 @@ module Gitlab
# user - User record
#
# Returns a Theme
def self.for_user(user)
def for_user(user)
if user
by_id(user.theme_id)
else
......@@ -71,7 +73,7 @@ module Gitlab
private
def self.default_id
def default_id
id = Gitlab.config.gitlab.default_theme.to_i
# Prevent an invalid configuration setting from causing an infinite loop
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment