From 5c7a738105b88c52d7765791419ee398d9e27ae0 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 8 Mar 2018 12:56:54 +0000 Subject: [CE] Add Naming/FileName rule checking expected class/module per filename --- .rubocop.yml | 72 ++++++++++++++ app/models/cycle_analytics/summary.rb | 0 config/routes/git_http.rb | 2 +- config/routes/group.rb | 4 +- config/routes/project.rb | 4 +- config/routes/user.rb | 4 +- lib/constraints/group_url_constrainer.rb | 12 ++- lib/constraints/project_url_constrainer.rb | 20 ++-- lib/constraints/user_url_constrainer.rb | 12 ++- lib/declarative_policy.rb | 4 +- lib/declarative_policy/delegate_dsl.rb | 16 ++++ lib/declarative_policy/dsl.rb | 103 --------------------- lib/declarative_policy/policy_dsl.rb | 44 +++++++++ lib/declarative_policy/preferred_scope.rb | 2 +- lib/declarative_policy/rule_dsl.rb | 45 +++++++++ lib/gitlab/auth/result.rb | 2 +- lib/gitlab/ci/pipeline/chain/command.rb | 2 +- lib/gitlab/health_checks/metric.rb | 2 +- lib/gitlab/health_checks/result.rb | 2 +- lib/gitlab/middleware/release_env.rb | 2 +- lib/gitlab/slash_commands/result.rb | 2 +- lib/haml_lint/inline_javascript.rb | 2 +- qa/qa/page/project/settings/ci_cd.rb | 2 +- rubocop/rubocop.rb | 1 + spec/lib/constraints/group_url_constrainer_spec.rb | 2 +- .../constraints/project_url_constrainer_spec.rb | 2 +- spec/lib/constraints/user_url_constrainer_spec.rb | 2 +- spec/routing/routing_spec.rb | 4 +- 28 files changed, 224 insertions(+), 147 deletions(-) delete mode 100644 app/models/cycle_analytics/summary.rb create mode 100644 lib/declarative_policy/delegate_dsl.rb delete mode 100644 lib/declarative_policy/dsl.rb create mode 100644 lib/declarative_policy/policy_dsl.rb create mode 100644 lib/declarative_policy/rule_dsl.rb diff --git a/.rubocop.yml b/.rubocop.yml index 293f61fb725..14840ddd262 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -31,6 +31,78 @@ Style/MutableConstant: - 'ee/db/post_migrate/**/*' - 'ee/db/geo/migrate/**/*' +Naming/FileName: + ExpectMatchingDefinition: true + Exclude: + - 'spec/**/*' + - 'features/**/*' + - 'ee/spec/**/*' + - 'qa/spec/**/*' + - 'qa/qa/specs/**/*' + - 'qa/bin/*' + - 'config/**/*' + - 'lib/generators/**/*' + - 'ee/lib/generators/**/*' + IgnoreExecutableScripts: true + AllowedAcronyms: + - EE + - JSON + - LDAP + - IO + - HMAC + - QA + - ENV + - STL + - PDF + - SVG + - CTE + - DN + - RSA + - CI + - CD + - OAuth + # default ones: + - CLI + - DSL + - ACL + - API + - ASCII + - CPU + - CSS + - DNS + - EOF + - GUID + - HTML + - HTTP + - HTTPS + - ID + - IP + - JSON + - LHS + - QPS + - RAM + - RHS + - RPC + - SLA + - SMTP + - SQL + - SSH + - TCP + - TLS + - TTL + - UDP + - UI + - UID + - UUID + - URI + - URL + - UTF8 + - VM + - XML + - XMPP + - XSRF + - XSS + # Gitlab ################################################################### Gitlab/ModuleWithInstanceVariables: diff --git a/app/models/cycle_analytics/summary.rb b/app/models/cycle_analytics/summary.rb deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/config/routes/git_http.rb b/config/routes/git_http.rb index ff51823897d..ec5c68f81df 100644 --- a/config/routes/git_http.rb +++ b/config/routes/git_http.rb @@ -40,7 +40,7 @@ scope(path: '*namespace_id/:project_id', # /info/refs?service=git-receive-pack, but nothing else. # git_http_handshake = lambda do |request| - ProjectUrlConstrainer.new.matches?(request) && + ::Constraints::ProjectUrlConstrainer.new.matches?(request) && (request.query_string.blank? || request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/)) end diff --git a/config/routes/group.rb b/config/routes/group.rb index 710f12e33ad..d89a714c7d6 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -1,10 +1,8 @@ -require 'constraints/group_url_constrainer' - resources :groups, only: [:index, :new, :create] do post :preview_markdown end -constraints(GroupUrlConstrainer.new) do +constraints(::Constraints::GroupUrlConstrainer.new) do scope(path: 'groups/*id', controller: :groups, constraints: { id: Gitlab::PathRegex.full_namespace_route_regex, format: /(html|json|atom)/ }) do diff --git a/config/routes/project.rb b/config/routes/project.rb index 710fe0ec325..b82ed27664c 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -1,10 +1,8 @@ -require 'constraints/project_url_constrainer' - resources :projects, only: [:index, :new, :create] draw :git_http -constraints(ProjectUrlConstrainer.new) do +constraints(::Constraints::ProjectUrlConstrainer.new) do # If the route has a wildcard segment, the segment has a regex constraint, # the segment is potentially followed by _another_ wildcard segment, and # the `format` option is not set to false, we need to specify that diff --git a/config/routes/user.rb b/config/routes/user.rb index 733a3f6ce9a..57fb37530bb 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -1,5 +1,3 @@ -require 'constraints/user_url_constrainer' - devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, registrations: :registrations, passwords: :passwords, @@ -35,7 +33,7 @@ scope(constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }) d get '/u/:username/contributed', to: redirect('users/%{username}/contributed') end -constraints(UserUrlConstrainer.new) do +constraints(::Constraints::UserUrlConstrainer.new) do # Get all keys of user get ':username.keys' => 'profiles/keys#get_keys', constraints: { username: Gitlab::PathRegex.root_namespace_route_regex } diff --git a/lib/constraints/group_url_constrainer.rb b/lib/constraints/group_url_constrainer.rb index fd2ac2db0a9..87649c50424 100644 --- a/lib/constraints/group_url_constrainer.rb +++ b/lib/constraints/group_url_constrainer.rb @@ -1,9 +1,11 @@ -class GroupUrlConstrainer - def matches?(request) - full_path = request.params[:group_id] || request.params[:id] +module Constraints + class GroupUrlConstrainer + def matches?(request) + full_path = request.params[:group_id] || request.params[:id] - return false unless NamespacePathValidator.valid_path?(full_path) + return false unless NamespacePathValidator.valid_path?(full_path) - Group.find_by_full_path(full_path, follow_redirects: request.get?).present? + Group.find_by_full_path(full_path, follow_redirects: request.get?).present? + end end end diff --git a/lib/constraints/project_url_constrainer.rb b/lib/constraints/project_url_constrainer.rb index e90ecb5ec69..32aea98f0f7 100644 --- a/lib/constraints/project_url_constrainer.rb +++ b/lib/constraints/project_url_constrainer.rb @@ -1,13 +1,15 @@ -class ProjectUrlConstrainer - def matches?(request) - namespace_path = request.params[:namespace_id] - project_path = request.params[:project_id] || request.params[:id] - full_path = [namespace_path, project_path].join('/') +module Constraints + class ProjectUrlConstrainer + def matches?(request) + namespace_path = request.params[:namespace_id] + project_path = request.params[:project_id] || request.params[:id] + full_path = [namespace_path, project_path].join('/') - return false unless ProjectPathValidator.valid_path?(full_path) + return false unless ProjectPathValidator.valid_path?(full_path) - # We intentionally allow SELECT(*) here so result of this query can be used - # as cache for further Project.find_by_full_path calls within request - Project.find_by_full_path(full_path, follow_redirects: request.get?).present? + # We intentionally allow SELECT(*) here so result of this query can be used + # as cache for further Project.find_by_full_path calls within request + Project.find_by_full_path(full_path, follow_redirects: request.get?).present? + end end end diff --git a/lib/constraints/user_url_constrainer.rb b/lib/constraints/user_url_constrainer.rb index 3b3ed1c6ddb..8afa04d29a4 100644 --- a/lib/constraints/user_url_constrainer.rb +++ b/lib/constraints/user_url_constrainer.rb @@ -1,9 +1,11 @@ -class UserUrlConstrainer - def matches?(request) - full_path = request.params[:username] +module Constraints + class UserUrlConstrainer + def matches?(request) + full_path = request.params[:username] - return false unless NamespacePathValidator.valid_path?(full_path) + return false unless NamespacePathValidator.valid_path?(full_path) - User.find_by_full_path(full_path, follow_redirects: request.get?).present? + User.find_by_full_path(full_path, follow_redirects: request.get?).present? + end end end diff --git a/lib/declarative_policy.rb b/lib/declarative_policy.rb index b1949d693ad..1dd2855063d 100644 --- a/lib/declarative_policy.rb +++ b/lib/declarative_policy.rb @@ -1,6 +1,8 @@ require_dependency 'declarative_policy/cache' require_dependency 'declarative_policy/condition' -require_dependency 'declarative_policy/dsl' +require_dependency 'declarative_policy/delegate_dsl' +require_dependency 'declarative_policy/policy_dsl' +require_dependency 'declarative_policy/rule_dsl' require_dependency 'declarative_policy/preferred_scope' require_dependency 'declarative_policy/rule' require_dependency 'declarative_policy/runner' diff --git a/lib/declarative_policy/delegate_dsl.rb b/lib/declarative_policy/delegate_dsl.rb new file mode 100644 index 00000000000..f544dffe888 --- /dev/null +++ b/lib/declarative_policy/delegate_dsl.rb @@ -0,0 +1,16 @@ +module DeclarativePolicy + # Used when the name of a delegate is mentioned in + # the rule DSL. + class DelegateDsl + def initialize(rule_dsl, delegate_name) + @rule_dsl = rule_dsl + @delegate_name = delegate_name + end + + def method_missing(m, *a, &b) + return super unless a.empty? && !block_given? + + @rule_dsl.delegate(@delegate_name, m) + end + end +end diff --git a/lib/declarative_policy/dsl.rb b/lib/declarative_policy/dsl.rb deleted file mode 100644 index 6ba1e7a3c5c..00000000000 --- a/lib/declarative_policy/dsl.rb +++ /dev/null @@ -1,103 +0,0 @@ -module DeclarativePolicy - # The DSL evaluation context inside rule { ... } blocks. - # Responsible for creating and combining Rule objects. - # - # See Base.rule - class RuleDsl - def initialize(context_class) - @context_class = context_class - end - - def can?(ability) - Rule::Ability.new(ability) - end - - def all?(*rules) - Rule::And.make(rules) - end - - def any?(*rules) - Rule::Or.make(rules) - end - - def none?(*rules) - ~Rule::Or.new(rules) - end - - def cond(condition) - Rule::Condition.new(condition) - end - - def delegate(delegate_name, condition) - Rule::DelegatedCondition.new(delegate_name, condition) - end - - def method_missing(m, *a, &b) - return super unless a.size == 0 && !block_given? - - if @context_class.delegations.key?(m) - DelegateDsl.new(self, m) - else - cond(m.to_sym) - end - end - end - - # Used when the name of a delegate is mentioned in - # the rule DSL. - class DelegateDsl - def initialize(rule_dsl, delegate_name) - @rule_dsl = rule_dsl - @delegate_name = delegate_name - end - - def method_missing(m, *a, &b) - return super unless a.size == 0 && !block_given? - - @rule_dsl.delegate(@delegate_name, m) - end - end - - # The return value of a rule { ... } declaration. - # Can call back to register rules with the containing - # Policy class (context_class here). See Base.rule - # - # Note that the #policy method just performs an #instance_eval, - # which is useful for multiple #enable or #prevent callse. - # - # Also provides a #method_missing proxy to the context - # class's class methods, so that helper methods can be - # defined and used in a #policy { ... } block. - class PolicyDsl - def initialize(context_class, rule) - @context_class = context_class - @rule = rule - end - - def policy(&b) - instance_eval(&b) - end - - def enable(*abilities) - @context_class.enable_when(abilities, @rule) - end - - def prevent(*abilities) - @context_class.prevent_when(abilities, @rule) - end - - def prevent_all - @context_class.prevent_all_when(@rule) - end - - def method_missing(m, *a, &b) - return super unless @context_class.respond_to?(m) - - @context_class.__send__(m, *a, &b) # rubocop:disable GitlabSecurity/PublicSend - end - - def respond_to_missing?(m) - @context_class.respond_to?(m) || super - end - end -end diff --git a/lib/declarative_policy/policy_dsl.rb b/lib/declarative_policy/policy_dsl.rb new file mode 100644 index 00000000000..f11b6e9f730 --- /dev/null +++ b/lib/declarative_policy/policy_dsl.rb @@ -0,0 +1,44 @@ +module DeclarativePolicy + # The return value of a rule { ... } declaration. + # Can call back to register rules with the containing + # Policy class (context_class here). See Base.rule + # + # Note that the #policy method just performs an #instance_eval, + # which is useful for multiple #enable or #prevent callse. + # + # Also provides a #method_missing proxy to the context + # class's class methods, so that helper methods can be + # defined and used in a #policy { ... } block. + class PolicyDsl + def initialize(context_class, rule) + @context_class = context_class + @rule = rule + end + + def policy(&b) + instance_eval(&b) + end + + def enable(*abilities) + @context_class.enable_when(abilities, @rule) + end + + def prevent(*abilities) + @context_class.prevent_when(abilities, @rule) + end + + def prevent_all + @context_class.prevent_all_when(@rule) + end + + def method_missing(m, *a, &b) + return super unless @context_class.respond_to?(m) + + @context_class.__send__(m, *a, &b) # rubocop:disable GitlabSecurity/PublicSend + end + + def respond_to_missing?(m) + @context_class.respond_to?(m) || super + end + end +end diff --git a/lib/declarative_policy/preferred_scope.rb b/lib/declarative_policy/preferred_scope.rb index b0754098149..5c214408dd0 100644 --- a/lib/declarative_policy/preferred_scope.rb +++ b/lib/declarative_policy/preferred_scope.rb @@ -1,4 +1,4 @@ -module DeclarativePolicy +module DeclarativePolicy # rubocop:disable Naming/FileName PREFERRED_SCOPE_KEY = :"DeclarativePolicy.preferred_scope" class << self diff --git a/lib/declarative_policy/rule_dsl.rb b/lib/declarative_policy/rule_dsl.rb new file mode 100644 index 00000000000..e948b7f2de1 --- /dev/null +++ b/lib/declarative_policy/rule_dsl.rb @@ -0,0 +1,45 @@ +module DeclarativePolicy + # The DSL evaluation context inside rule { ... } blocks. + # Responsible for creating and combining Rule objects. + # + # See Base.rule + class RuleDsl + def initialize(context_class) + @context_class = context_class + end + + def can?(ability) + Rule::Ability.new(ability) + end + + def all?(*rules) + Rule::And.make(rules) + end + + def any?(*rules) + Rule::Or.make(rules) + end + + def none?(*rules) + ~Rule::Or.new(rules) + end + + def cond(condition) + Rule::Condition.new(condition) + end + + def delegate(delegate_name, condition) + Rule::DelegatedCondition.new(delegate_name, condition) + end + + def method_missing(m, *a, &b) + return super unless a.empty? && !block_given? + + if @context_class.delegations.key?(m) + DelegateDsl.new(self, m) + else + cond(m.to_sym) + end + end + end +end diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb index 75451cf8aa9..00cdc94a9ef 100644 --- a/lib/gitlab/auth/result.rb +++ b/lib/gitlab/auth/result.rb @@ -1,4 +1,4 @@ -module Gitlab +module Gitlab # rubocop:disable Naming/FileName module Auth Result = Struct.new(:actor, :project, :type, :authentication_abilities) do def ci?(for_project) diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 7b19b10e05b..a1849b01c5d 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -1,4 +1,4 @@ -module Gitlab +module Gitlab # rubocop:disable Naming/FileName module Ci module Pipeline module Chain diff --git a/lib/gitlab/health_checks/metric.rb b/lib/gitlab/health_checks/metric.rb index 1a2eab0b005..d62d9136886 100644 --- a/lib/gitlab/health_checks/metric.rb +++ b/lib/gitlab/health_checks/metric.rb @@ -1,3 +1,3 @@ -module Gitlab::HealthChecks +module Gitlab::HealthChecks # rubocop:disable Naming/FileName Metric = Struct.new(:name, :value, :labels) end diff --git a/lib/gitlab/health_checks/result.rb b/lib/gitlab/health_checks/result.rb index 8086760023e..e323e2c9723 100644 --- a/lib/gitlab/health_checks/result.rb +++ b/lib/gitlab/health_checks/result.rb @@ -1,3 +1,3 @@ -module Gitlab::HealthChecks +module Gitlab::HealthChecks # rubocop:disable Naming/FileName Result = Struct.new(:success, :message, :labels) end diff --git a/lib/gitlab/middleware/release_env.rb b/lib/gitlab/middleware/release_env.rb index f8d0a135965..bfe8e113b5e 100644 --- a/lib/gitlab/middleware/release_env.rb +++ b/lib/gitlab/middleware/release_env.rb @@ -1,4 +1,4 @@ -module Gitlab +module Gitlab # rubocop:disable Naming/FileName module Middleware # Some of middleware would hold env for no good reason even after the # request had already been processed, and we could not garbage collect diff --git a/lib/gitlab/slash_commands/result.rb b/lib/gitlab/slash_commands/result.rb index 7021b4b01b2..3669dedf0fe 100644 --- a/lib/gitlab/slash_commands/result.rb +++ b/lib/gitlab/slash_commands/result.rb @@ -1,4 +1,4 @@ -module Gitlab +module Gitlab # rubocop:disable Naming/FileName module SlashCommands Result = Struct.new(:type, :message) end diff --git a/lib/haml_lint/inline_javascript.rb b/lib/haml_lint/inline_javascript.rb index 4f776330e80..adbed20f152 100644 --- a/lib/haml_lint/inline_javascript.rb +++ b/lib/haml_lint/inline_javascript.rb @@ -1,4 +1,4 @@ -unless Rails.env.production? +unless Rails.env.production? # rubocop:disable Naming/FileName require 'haml_lint/haml_visitor' require 'haml_lint/linter' require 'haml_lint/linter_registry' diff --git a/qa/qa/page/project/settings/ci_cd.rb b/qa/qa/page/project/settings/ci_cd.rb index 99be21bbe89..17a1bc904e4 100644 --- a/qa/qa/page/project/settings/ci_cd.rb +++ b/qa/qa/page/project/settings/ci_cd.rb @@ -1,4 +1,4 @@ -module QA +module QA # rubocop:disable Naming/FileName module Page module Project module Settings diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 9110237c538..b36a3f9c8a0 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,3 +1,4 @@ +# rubocop:disable Naming/FileName require_relative 'cop/gitlab/module_with_instance_variables' require_relative 'cop/gitlab/predicate_memoization' require_relative 'cop/include_sidekiq_worker' diff --git a/spec/lib/constraints/group_url_constrainer_spec.rb b/spec/lib/constraints/group_url_constrainer_spec.rb index 4dab58b26a0..ff295068ba9 100644 --- a/spec/lib/constraints/group_url_constrainer_spec.rb +++ b/spec/lib/constraints/group_url_constrainer_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe GroupUrlConstrainer do +describe Constraints::GroupUrlConstrainer do let!(:group) { create(:group, path: 'gitlab') } describe '#matches?' do diff --git a/spec/lib/constraints/project_url_constrainer_spec.rb b/spec/lib/constraints/project_url_constrainer_spec.rb index 92331eb2e5d..c96e7ab8495 100644 --- a/spec/lib/constraints/project_url_constrainer_spec.rb +++ b/spec/lib/constraints/project_url_constrainer_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe ProjectUrlConstrainer do +describe Constraints::ProjectUrlConstrainer do let!(:project) { create(:project) } let!(:namespace) { project.namespace } diff --git a/spec/lib/constraints/user_url_constrainer_spec.rb b/spec/lib/constraints/user_url_constrainer_spec.rb index cb3b4ff1391..e2c85bb27bb 100644 --- a/spec/lib/constraints/user_url_constrainer_spec.rb +++ b/spec/lib/constraints/user_url_constrainer_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe UserUrlConstrainer do +describe Constraints::UserUrlConstrainer do let!(:user) { create(:user, username: 'dz') } describe '#matches?' do diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 56d025f0176..9345671a1a7 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -9,7 +9,7 @@ require 'spec_helper' # user_calendar_activities GET /u/:username/calendar_activities(.:format) describe UsersController, "routing" do it "to #show" do - allow_any_instance_of(UserUrlConstrainer).to receive(:matches?).and_return(true) + allow_any_instance_of(::Constraints::UserUrlConstrainer).to receive(:matches?).and_return(true) expect(get("/User")).to route_to('users#show', username: 'User') end @@ -210,7 +210,7 @@ describe Profiles::KeysController, "routing" do # get all the ssh-keys of a user it "to #get_keys" do - allow_any_instance_of(UserUrlConstrainer).to receive(:matches?).and_return(true) + allow_any_instance_of(::Constraints::UserUrlConstrainer).to receive(:matches?).and_return(true) expect(get("/foo.keys")).to route_to('profiles/keys#get_keys', username: 'foo') end -- cgit v1.2.1