Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@ jobs:
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
ruby-version: '3.3'
bundler: latest
bundler-cache: true
- run: bundle exec rubocop
3 changes: 2 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
inherit_from: .rubocop_todo.yml

require:
plugins:
- rubocop-performance
- rubocop-rake
- rubocop-rspec
- rubocop-exception_messages

AllCops:
TargetRubyVersion: 2.7
Expand Down
15 changes: 13 additions & 2 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Remaining offenses after safe autocorrection with modern RuboCop.
Naming/PredicateMethod:
Exclude:
- 'lib/slack/events/request.rb'
- 'lib/slack/utils/security.rb'

Style/DirectiveScope:
Exclude:
- 'lib/slack/utils/security.rb'

# Offense count: 1
# This cop supports unsafe auto-correction (--auto-correct-all).
Lint/NonDeterministicRequireOrder:
Expand Down Expand Up @@ -107,7 +117,7 @@ RSpec/ExampleLength:
# Offense count: 10
# Configuration parameters: Include, CustomTransform, IgnoreMethods, SpecSuffixOnly.
# Include: **/*_spec*rb*, **/spec/**/*
RSpec/FilePath:
RSpec/SpecFilePathFormat:
Exclude:
- 'spec/slack/version_spec.rb'
- 'spec/slack/web/api/endpoints/custom/auth_spec.rb'
Expand Down Expand Up @@ -163,7 +173,8 @@ RSpec/SubjectStub:
# Configuration parameters: .
# SupportedStyles: constant, string
RSpec/VerifiedDoubleReference:
EnforcedStyle: string
Exclude:
- 'spec/slack/web/faraday/request_spec.rb'

# Offense count: 2
# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
Expand Down
13 changes: 13 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Contributor guidance

Read [CONTRIBUTING.md](CONTRIBUTING.md) first for setup, tests, lint, changelog, pull request, and API patch instructions. Keep those instructions there rather than duplicating them here.

- Preserve the supported-Ruby test matrix when changing lint dependencies.
- When upgrading lint tooling, review safe corrections and baseline only remaining unrelated offenses.
- Exception message corrections can change public error strings. Update matching specs, including handwritten specs under `spec/slack/web/api/endpoints/custom/`.
- Use lowercase exception messages without a trailing period, including messages passed to `super` in error constructors; the exception-message cops may not detect that form. Preserve upstream Slack error codes and dynamically supplied messages.
- Load RuboCop extensions through `plugins:`. When upgrading an extension, migrate renamed cops and obsolete parameters, review safe corrections, and preserve existing behavior rather than blindly applying unsafe fixes.
- Do not edit the API-reference submodule or include existing submodule changes unless requested.
- The API update task pulls the latest reference and can generate unrelated changes. Verify the patch against both the latest and pinned reference; keep unrelated generated updates out of a focused PR. Custom endpoint specs are handwritten despite their location under `endpoints/`.
- Do not suppress environment warnings in application code or tests. Report local validation limitations explicitly; CLI specs compare subprocess output and can fail when Bundler emits unrelated warnings.
- Keep the test bundle on JSON below 3 until Faraday's JSON middleware supports JSON 3's keyword-only parse options.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### 3.3.0 (Next)

* [#603](https://github.com/slack-ruby/slack-ruby-client/pull/603): Enforce consistent exception messages with rubocop-exception_messages and run lint separately from the test matrix - [@dblock](https://github.com/dblock).
* [#602](https://github.com/slack-ruby/slack-ruby-client/pull/602): Make gli an optional dependency; only the `slack` command-line client needs it - [@corsonknowles](https://github.com/corsonknowles).
* [#599](https://github.com/slack-ruby/slack-ruby-client/pull/599): Set a default filename in `files_upload` so Slack displays image previews correctly when none is specified - [@ts-3156](https://github.com/ts-3156), [@dblock](https://github.com/dblock).
* [#591](https://github.com/slack-ruby/slack-ruby-client/pull/591): Generate AI CHANGELOG entries and PR summaries for automated API update PRs, lock simplecov below 1.1.0 to avoid breaking Coveralls - [@dblock](https://github.com/dblock).
Expand Down
11 changes: 10 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,16 @@ We definitely appreciate pull requests that highlight or reproduce a problem, ev

Implement your feature or bug fix.

Ruby style is enforced with [Rubocop](https://github.com/bbatsov/rubocop). Run `bundle exec rubocop` and fix any style issues highlighted, auto-correct issues when possible with `bundle exec rubocop -a`. To silence generally ignored issues, including line lengths or code complexity metrics, run `bundle exec rubocop --auto-gen-config`.
Ruby style is enforced with [RuboCop](https://github.com/rubocop/rubocop), including [exception message conventions](https://github.com/dblock/rubocop-exception_messages). The main Gemfile includes lint dependencies only on Ruby 3.2+ so tests can continue running on older supported Ruby versions.

```bash
bundle install
bundle exec rubocop
```

Fix any style issues highlighted, auto-correcting safe issues with `bundle exec rubocop -a`. Exception message corrections require `-A`; review them and update message expectations in tests. To regenerate the baseline for generally ignored issues, including line lengths or code complexity metrics, run `bundle exec rubocop --auto-gen-config`.

On Ruby 3.2+, `bundle exec rake rubocop` also runs lint. The default `bundle exec rake` runs tests only; CI runs lint separately on Ruby 3.3.

Make sure that `bundle exec rake` completes without errors.

Expand Down
15 changes: 11 additions & 4 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,12 @@ group :test do
gem 'erubis'
gem 'faraday-typhoeus'
gem 'gli'
gem 'json', '< 3' # Faraday's JSON middleware passes options as a positional hash.
gem 'json-schema'
gem 'mutex_m'
gem 'racc'
gem 'rake', '~> 13'
gem 'rspec'
gem 'rubocop', '1.26.1' # Lock to specific version to avoid breaking cops/changes
gem 'rubocop-performance'
gem 'rubocop-rake'
gem 'rubocop-rspec'
# Lock below 1.1.0, which started writing float timestamps to
# coverage/.resultset.json, breaking coverallsapp/github-action's parser
# (coverallsapp/github-action#269, coverallsapp/coverage-reporter#191).
Expand All @@ -31,3 +28,13 @@ group :test do
gem 'vcr'
gem 'webmock'
end

if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.2')
group :lint do
gem 'rubocop', '~> 1.72'
gem 'rubocop-exception_messages', '~> 0.2.0', require: false
gem 'rubocop-performance', '~> 1.27'
gem 'rubocop-rake', '~> 0.7'
gem 'rubocop-rspec', '~> 3.10'
end
end
10 changes: 7 additions & 3 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ RSpec::Core::RakeTask.new(:spec) do |spec|
spec.pattern = FileList['spec/**/*_spec.rb']
end

require 'rubocop/rake_task'
RuboCop::RakeTask.new
desc 'Run RuboCop (Ruby 3.2+).'
task :rubocop do
abort 'RuboCop requires Ruby 3.2 or newer' if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('3.2')

task default: %i[spec rubocop]
sh 'bundle exec rubocop'
end

task default: :spec

load 'tasks/git.rake'
load 'tasks/web.rake'
Expand Down
2 changes: 1 addition & 1 deletion examples/files_upload_v2/files_upload_v2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require 'securerandom'

Slack.configure do |config|
config.token = ENV['SLACK_API_TOKEN']
config.token = ENV.fetch('SLACK_API_TOKEN', nil)
raise 'Missing ENV[SLACK_API_TOKEN]!' unless config.token
end

Expand Down
2 changes: 1 addition & 1 deletion examples/hi_web/hi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
require 'slack-ruby-client'

Slack.configure do |config|
config.token = ENV['SLACK_API_TOKEN']
config.token = ENV.fetch('SLACK_API_TOKEN', nil)
raise 'Missing ENV[SLACK_API_TOKEN]!' unless config.token
end

Expand Down
2 changes: 1 addition & 1 deletion examples/new_ticket/new_ticket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
require 'slack-ruby-client'

Slack.configure do |config|
config.token = ENV['SLACK_API_TOKEN']
config.token = ENV.fetch('SLACK_API_TOKEN', nil)
raise 'Missing ENV[SLACK_API_TOKEN]!' unless config.token
end

Expand Down
12 changes: 6 additions & 6 deletions examples/oauth_v2/oauth_v2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

response = client.oauth_v2_access(
req.query.merge(
client_id: ENV['SLACK_CLIENT_ID'],
client_secret: ENV['SLACK_CLIENT_SECRET'],
client_id: ENV.fetch('SLACK_CLIENT_ID', nil),
client_secret: ENV.fetch('SLACK_CLIENT_SECRET', nil),
grant_type: 'authorization_code'
)
)
Expand Down Expand Up @@ -47,10 +47,10 @@
end

query = {
client_id: ENV['SLACK_CLIENT_ID'],
redirect_uri: ENV['REDIRECT_URI'],
scope: ENV['SCOPE'],
user_scope: ENV['USER_SCOPE']
client_id: ENV.fetch('SLACK_CLIENT_ID', nil),
redirect_uri: ENV.fetch('REDIRECT_URI', nil),
scope: ENV.fetch('SCOPE', nil),
user_scope: ENV.fetch('USER_SCOPE', nil)
}

url = "https://slack.com/oauth/v2/authorize?#{query.to_query}"
Expand Down
2 changes: 1 addition & 1 deletion lib/slack/events/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Config
attr_accessor(*Config::ATTRIBUTES)

def reset
self.signing_secret = ENV['SLACK_SIGNING_SECRET']
self.signing_secret = ENV.fetch('SLACK_SIGNING_SECRET', nil)
self.signature_expires_in = 5 * 60
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/slack/web/api/errors/server_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ServerError < ::Faraday::Error

def initialize(message, response)
@response = response
super message
super(message)
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/slack/web/api/errors/slack_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class SlackError < ::Faraday::Error
attr_reader :response

def initialize(message, response = nil)
super message
super(message)
@response = response
end

Expand Down
2 changes: 1 addition & 1 deletion lib/slack/web/api/errors/too_many_requests_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class TooManyRequestsError < ::Faraday::Error

def initialize(response)
@response = response
super "Retry after #{retry_after} seconds"
super("retry after #{retry_after} seconds")
end

def retry_after
Expand Down
4 changes: 2 additions & 2 deletions lib/slack/web/api/helpers/files.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ def files_upload_v2(params = {})

files_to_upload.each_with_index do |file, index|
%i[filename content].each do |param|
raise ArgumentError, "Required argument :#{param} missing in file (#{index})" if file[param].nil?
raise ArgumentError, "required argument :`#{param}` missing in file (`#{index}`)" if file[param].nil?
end
end

channel_params = %i[channel channels channel_id].map { |param| params[param] }.compact
raise ArgumentError, 'Only one of :channel, :channels, or :channel_id is required' if channel_params.size > 1
raise ArgumentError, 'only one of :channel, :channels, or :channel_id is required' if channel_params.size > 1

complete_upload_request_params = params.slice(:initial_comment, :blocks, :thread_ts)

Expand Down
3 changes: 2 additions & 1 deletion lib/slack/web/api/mixins/conversations.id.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Api
module Mixins
module Conversations
include Ids

#
# This method returns a channel ID given a channel name.
#
Expand All @@ -24,7 +25,7 @@ module Conversations
# containing one or more of public_channel, private_channel, mpim, im
def conversations_id(options = {})
name = options[:channel]
raise ArgumentError, 'Required arguments :channel missing' if name.nil?
raise ArgumentError, 'required arguments :channel missing' if name.nil?

id_for(
key: :channel,
Expand Down
3 changes: 2 additions & 1 deletion lib/slack/web/api/mixins/users.id.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Api
module Mixins
module Users
include Ids

#
# This method returns a user ID given a user name.
#
Expand All @@ -19,7 +20,7 @@ module Users
# The page size used for users_list calls required to find the user's ID
def users_id(options = {})
name = options[:user]
raise ArgumentError, 'Required arguments :user missing' if name.nil?
raise ArgumentError, 'required arguments :user missing' if name.nil?

id_for(
key: :user,
Expand Down
2 changes: 1 addition & 1 deletion lib/slack/web/api/mixins/users.search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module Users
# Free-formed text to search for.
def users_search(options = {})
query = options[:user]
raise ArgumentError, 'Required arguments :user missing' if query.nil?
raise ArgumentError, 'required arguments :user missing' if query.nil?

index = Picky::Index.new(:users) do
category :name
Expand Down
2 changes: 1 addition & 1 deletion spec/slack/web/api/endpoints/custom/auth_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
it 'fails with an specific exception' do
client.auth_test
rescue Slack::Web::Api::Errors::TooManyRequestsError => e
expect(e.message).to eq('Retry after 3600 seconds')
expect(e.message).to eq('retry after 3600 seconds')
expect(e.retry_after).to eq(3600)
end
end
Expand Down
10 changes: 5 additions & 5 deletions spec/slack/web/api/endpoints/custom/files_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
it 'raises an argument error' do
expect do
client.files_upload_v2(params)
end.to raise_error ArgumentError, /Required argument :#{arg} missing/
end.to raise_error ArgumentError, /required argument :`#{arg}` missing/
end
end
end
Expand All @@ -61,7 +61,7 @@
filename: 'test.txt',
content: 'Test File Contents'
}.merge(permutation.map { |arg| [arg, 'C08AZ76CA4V'] }.to_h))
end.to raise_error ArgumentError, 'Only one of :channel, :channels, or :channel_id is required'
end.to raise_error ArgumentError, 'only one of :channel, :channels, or :channel_id is required'
end
end

Expand All @@ -73,7 +73,7 @@
channels: 'C08AZ76CA4V',
channel_id: 'C08AZ76CA4V'
)
end.to raise_error ArgumentError, 'Only one of :channel, :channels, or :channel_id is required'
end.to raise_error ArgumentError, 'only one of :channel, :channels, or :channel_id is required'
end

%i[channel channels].each do |arg|
Expand Down Expand Up @@ -205,7 +205,7 @@
],
channel_id: 'C04KB5X4D'
)
end.to raise_error ArgumentError, /Required argument :filename missing in file \(0\)/
end.to raise_error ArgumentError, /required argument :`filename` missing in file \(`0`\)/
end
end

Expand All @@ -219,7 +219,7 @@
],
channel_id: 'C04KB5X4D'
)
end.to raise_error ArgumentError, /Required argument :content missing in file \(0\)/
end.to raise_error ArgumentError, /required argument :`content` missing in file \(`0`\)/
end
end
end
34 changes: 34 additions & 0 deletions spec/slack/web/api/helpers/files_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe Slack::Web::Api::Helpers::Files do
subject(:client) { Slack::Web::Client.new }

describe '#files_upload_v2' do
it 'requires a filename' do
expect { client.files_upload_v2(content: 'hello') }.to(
raise_error(ArgumentError, 'required argument :`filename` missing in file (`0`)')
)
end

it 'requires content' do
expect { client.files_upload_v2(filename: 'hello.txt') }.to(
raise_error(ArgumentError, 'required argument :`content` missing in file (`0`)')
)
end

it 'identifies the invalid file in a batch' do
files = [{ filename: 'hello.txt', content: 'hello' }, { content: 'world' }]
expect { client.files_upload_v2(files: files) }.to(
raise_error(ArgumentError, 'required argument :`filename` missing in file (`1`)')
)
end

it 'rejects conflicting channel arguments' do
expect do
client.files_upload_v2(filename: 'hello.txt', content: 'hello', channel: '#general', channel_id: 'C123456')
end.to raise_error(ArgumentError, 'only one of :channel, :channels, or :channel_id is required')
end
end
end
6 changes: 6 additions & 0 deletions spec/slack/web/api/mixins/conversations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
end

describe '#conversations_id' do
it 'requires a channel' do
expect { conversations.conversations_id }.to(
raise_error(ArgumentError, 'required arguments :channel missing')
)
end

it 'leaves channels specified by ID alone' do
expect(conversations.conversations_id(channel: 'C123456')).to(
eq('ok' => true, 'channel' => { 'id' => 'C123456' })
Expand Down
Loading
Loading