Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support unparenthesized Gemfile methods #358

Merged
merged 2 commits into from
Nov 20, 2022
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions fixtures/small/gemfile_actual.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
source "https://rubygems.org"
ruby "2.7.3"
gem "nokogiri"

source "https://my_special_hosted_thing.io" do
gem "my_hosted_gem", '~>1.2.3'
gem "rake", :require => false
end

group :test do
gem 'rspec'
end
12 changes: 12 additions & 0 deletions fixtures/small/gemfile_expected.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
source "https://rubygems.org"
ruby "2.7.3"
gem "nokogiri"

source "https://my_special_hosted_thing.io" do
gem "my_hosted_gem", "~>1.2.3"
gem "rake", :require => false
end

group :test do
gem "rspec"
end
1 change: 1 addition & 0 deletions librubyfmt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ serde_json = "1.0.40"
backtrace = "0.3.45"
libc = "0.2.68"
ripper_deserialize = { path = "ripper_deserialize" }
lazy_static = "1.4.0"
log = { version = "0.4.8", features = ["max_level_debug", "release_max_level_warn"] }
simplelog = "0.8"

Expand Down
40 changes: 34 additions & 6 deletions librubyfmt/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::HashSet;

use crate::delimiters::BreakableDelims;
use crate::heredoc_string::HeredocKind;
use crate::parser_state::{BaseParserState, ConcreteParserState, FormattingContext, RenderFunc};
Expand Down Expand Up @@ -661,6 +663,21 @@ pub fn args_has_single_def_expression(args: &ArgsAddStarOrExpressionListOrArgsFo
false
}

lazy_static! {
static ref RSPEC_METHODS: HashSet<&'static str> = vec!["it", "describe"].into_iter().collect();
static ref GEMFILE_METHODS: HashSet<&'static str> = vec![
// Gemfile
"gem",
"source",
"ruby",
"group",
].into_iter().collect();
static ref OPTIONALLY_PARENTHESIZED_METHODS: HashSet<&'static str> =
vec!["super", "require", "require_relative",]
.into_iter()
.collect::<HashSet<_>>();
}

pub fn use_parens_for_method_call(
ps: &dyn ConcreteParserState,
chain: &[CallChainElement],
Expand Down Expand Up @@ -702,7 +719,9 @@ pub fn use_parens_for_method_call(
}
}

if name == "super" || name == "require" || name == "require_relative" {
if OPTIONALLY_PARENTHESIZED_METHODS.contains(name.as_str())
|| GEMFILE_METHODS.contains(name.as_str())
{
return original_used_parens;
}

Expand Down Expand Up @@ -2663,18 +2682,27 @@ pub fn format_backref(ps: &mut dyn ConcreteParserState, backref: Backref) {
}
}

fn can_elide_parens_for_rspec_dsl_call(cc: &[CallChainElement]) -> bool {
/// Matches call chains on common special-cased names, like
/// `it`/`describe` for tests and `gem`/`source`/etc. for Gemfiles.
fn can_elide_parens_for_reserved_names(cc: &[CallChainElement]) -> bool {
if let Some(CallChainElement::Block(Block::BraceBlock(_))) = cc.last() {
return false;
};

let is_bare_it_or_describe = match cc.get(0) {
let is_bare_reserved_method_name = match cc.get(0) {
Some(CallChainElement::IdentOrOpOrKeywordOrConst(IdentOrOpOrKeywordOrConst::Ident(
Ident(_, ident, _),
))) => ident == "it" || ident == "describe",
))) => {
let ident = ident.as_str();
RSPEC_METHODS.contains(ident) || GEMFILE_METHODS.contains(ident)
}
_ => false,
};

if is_bare_reserved_method_name {
return true;
}

let is_rspec_describe = match (cc.get(0), cc.get(2)) {
(
Some(CallChainElement::VarRef(VarRef(_, VarRefType::Const(Const(_, c, _))))),
Expand All @@ -2685,7 +2713,7 @@ fn can_elide_parens_for_rspec_dsl_call(cc: &[CallChainElement]) -> bool {
_ => false,
};

is_bare_it_or_describe || is_rspec_describe
is_rspec_describe
}

/// Returns `true` if the call chain is indented, `false` if not
Expand All @@ -2709,7 +2737,7 @@ fn format_call_chain_elements(
cc: Vec<CallChainElement>,
render_multiline_chain: bool,
) {
let elide_parens = can_elide_parens_for_rspec_dsl_call(&cc);
let elide_parens = can_elide_parens_for_reserved_names(&cc);
let mut has_indented = false;
// When set, force all `CallChainElement::ArgsAddStarOrExpressionListOrArgsForward`
// to use parens, even when empty. This handles cases like `super()` where parens matter
Expand Down
3 changes: 3 additions & 0 deletions librubyfmt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ use std::io::{Cursor, Write};
use std::slice;
use std::str;

#[macro_use]
extern crate lazy_static;

#[cfg(all(feature = "use_jemalloc", not(target_env = "msvc")))]
#[global_allocator]
static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc;
Expand Down