Not removing commas from end of multiple attributes list

src/attr.rs

@@ -295,7 +295,8 @@ impl Rewrite for ast::MetaItem {
             }
             ast::MetaItemKind::List(ref list) => {
                 let path = rewrite_path(context, PathContext::Type, None, &self.path, shape)?;
-                let has_trailing_comma = crate::expr::span_ends_with_comma(context, self.span);
+                let has_trailing_comma =
+                    crate::expr::span_ends_with_comma(context, self.span, true);
                 overflow::rewrite_with_parens(
                     context,
                     &path,

src/expr.rs

@@ -1299,7 +1299,7 @@ fn rewrite_int_lit(context: &RewriteContext<'_>, lit: &ast::Lit, shape: Shape) -
 
 fn choose_separator_tactic(context: &RewriteContext<'_>, span: Span) -> Option<SeparatorTactic> {
     if context.inside_macro() {
-        if span_ends_with_comma(context, span) {
+        if span_ends_with_comma(context, span, false) {
             Some(SeparatorTactic::Always)
         } else {
             Some(SeparatorTactic::Never)
@@ -1417,16 +1417,27 @@ pub(crate) fn is_nested_call(expr: &ast::Expr) -> bool {
 /// Returns `true` if a function call or a method call represented by the given span ends with a
 /// trailing comma. This function is used when rewriting macro, as adding or removing a trailing
 /// comma from macro can potentially break the code.
-pub(crate) fn span_ends_with_comma(context: &RewriteContext<'_>, span: Span) -> bool {
+pub(crate) fn span_ends_with_comma(
+    context: &RewriteContext<'_>,
+    span: Span,
+    is_attributes: bool,
+) -> bool {
     let mut result: bool = Default::default();
     let mut prev_char: char = Default::default();
     let closing_delimiters = &[')', '}', ']'];
 
-    for (kind, c) in CharClasses::new(context.snippet(span).chars()) {
+    let snippet = context.snippet(span);
+    let mut len = snippet.len();
+    for (kind, c) in CharClasses::new(snippet.chars()) {
+        len -= 1;
         match c {
             _ if kind.is_comment() || c.is_whitespace() => continue,
             c if closing_delimiters.contains(&c) => {
-                result &= !closing_delimiters.contains(&prev_char);
+                // #3277 fix - for Attributes, ignore the closing ']', as in case of attributes
+                // list, the last AST item includes both the list ')' and attributes ']' closings.
+                if !is_attributes || len != 0 || c != ']' {
+                    result &= !closing_delimiters.contains(&prev_char);
+                }
             }
             ',' => result = true,
             _ => result = false,
@@ -1672,7 +1683,7 @@ fn rewrite_struct_lit<'a>(
         let tactic = struct_lit_tactic(h_shape, context, &item_vec);
         let nested_shape = shape_for_tactic(tactic, h_shape, v_shape);
 
-        let ends_with_comma = span_ends_with_comma(context, span);
+        let ends_with_comma = span_ends_with_comma(context, span, false);
         let force_no_trailing_comma = context.inside_macro() && !ends_with_comma;
 
         let fmt = struct_lit_formatting(
@@ -1843,7 +1854,7 @@ pub(crate) fn rewrite_tuple<'a, T: 'a + IntoOverflowableItem<'a>>(
     if context.use_block_indent() {
         // We use the same rule as function calls for rewriting tuples.
         let force_tactic = if context.inside_macro() {
-            if span_ends_with_comma(context, span) {
+            if span_ends_with_comma(context, span, false) {
                 Some(SeparatorTactic::Always)
             } else {
                 Some(SeparatorTactic::Never)

tests/source/issue-3277/one.rs

@@ -0,0 +1,160 @@
+// rustfmt-version: One
+
+// Original issue case
+#![allow(clippy::needless_pass_by_value, clippy::new_ret_no_self, clippy::new_without_default_derive,)]
+
+// Different cases with commas between parenthesis
+#[cfg(feature1 = "std1",feature2 = "std2",)]
+#[cfg(all(feature1 = "std1",feature2 = "std2",))]
+#[cfg(all(feature1 = "std1",feature2 = "std2",),)]
+#[cfg(all(all(feature1 = "std1",feature2 = "std2",),),)]
+#[cfg(all(all(feature1 = "std1",feature2 = "std2",)),)]
+#[cfg(all(all(feature1 = "std1",feature2 = "std2",)))]
+type Os = NoSource;
+
+// Comma at the end of the last list items
+#[live_prop_test(
+precondition = "inputs.input_flows.len() == self.num_inputs()",
+postcondition = "result.len() == self.num_outputs()",
+postcondition = "output_times_valid(inputs, &result)",
+)]
+fn output_flows(
+&self,
+inputs: MachineObservedInputs,
+future: &Self::Future,
+) -> Inputs<Option<MaterialFlow>> {
+inputs![]
+}
+
+#[cfg(all(feature1 = "std1",feature2 = "std2",))]
+#[cfg(all(feature1 = "long textttttttttttttttttttttttttttttttttttttttt",feature2 = "long textttttttttttttttttttttttttttttttttttttttt",))]
+#[cfg(not(all(
+feature1 = "long textttttttttttttttttttttttttttttttttttttttt",
+feature2 = "long textttttttttttttttttttttttttttttttttttttttt",
+)))]
+type Os = NoSource;
+
+#[cfg(not(all(
+feature = "std",
+any(target_os = "linux",
+target_os = "android",
+)
+)))]
+#[cfg(not(all(
+feature = "std",
+any(target_os = "linuxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+target_os = "android",
+)
+)))]
+#[cfg(not(all(
+any(
+target_os = "linux",
+target_os = "android",
+),
+feature = "std",
+)))]
+#[cfg(
+any(
+target_os = "linux",
+target_os = "android",
+)
+)]
+#[cfg(
+target_os = "linux",
+target_os = "android",
+)]
+#[cfg(not(all(
+target_os = "linux",
+target_os = "android",
+),
+feature = "std",
+))]    
+type Os = NoSource;
+
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows",
+)
+)
+)]
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows",
+),
+target_os = "unix",
+)
+)]
+type Os = NoSource;
+
+// Tests with no comma at the end of the last list items
+#[live_prop_test(
+precondition = "inputs.input_flows.len() == self.num_inputs()",
+postcondition = "result.len() == self.num_outputs()",
+postcondition = "output_times_valid(inputs, &result)"
+)]
+fn output_flows(
+&self,
+inputs: MachineObservedInputs,
+future: &Self::Future,
+) -> Inputs<Option<MaterialFlow>> {
+inputs![]
+}
+
+#[cfg(all(feature1 = "long textttttttttttttttttttttttttttttttttttttttt",feature2 = "long textttttttttttttttttttttttttttttttttttttttt"))]
+#[cfg(not(all(
+feature1 = "long textttttttttttttttttttttttttttttttttttttttt",
+feature2 = "long textttttttttttttttttttttttttttttttttttttttt"
+)))]
+type Os = NoSource;
+
+#[cfg(not(all(
+feature = "std",
+any(target_os = "linux",
+target_os = "android"
+)
+)))]
+#[cfg(not(all(
+any(
+target_os = "linux",
+target_os = "android"
+),
+feature = "std"
+)))]
+#[cfg(
+any(
+target_os = "linux",
+target_os = "android"
+)
+)]
+#[cfg(
+target_os = "linux",
+target_os = "android"
+)]
+type Os = NoSource;
+
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows"
+)
+)
+)]
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows"
+),
+target_os = "unix"
+)
+)]
+type Os = NoSource;

tests/source/issue-3277/two.rs

@@ -0,0 +1,160 @@
+// rustfmt-version: Two
+
+// Original issue case
+#![allow(clippy::needless_pass_by_value, clippy::new_ret_no_self, clippy::new_without_default_derive,)]
+
+// Different cases with commas between parenthesis
+#[cfg(feature1 = "std1",feature2 = "std2",)]
+#[cfg(all(feature1 = "std1",feature2 = "std2",))]
+#[cfg(all(feature1 = "std1",feature2 = "std2",),)]
+#[cfg(all(all(feature1 = "std1",feature2 = "std2",),),)]
+#[cfg(all(all(feature1 = "std1",feature2 = "std2",)),)]
+#[cfg(all(all(feature1 = "std1",feature2 = "std2",)))]
+type Os = NoSource;
+
+// Comma at the end of the last list items
+#[live_prop_test(
+precondition = "inputs.input_flows.len() == self.num_inputs()",
+postcondition = "result.len() == self.num_outputs()",
+postcondition = "output_times_valid(inputs, &result)",
+)]
+fn output_flows(
+&self,
+inputs: MachineObservedInputs,
+future: &Self::Future,
+) -> Inputs<Option<MaterialFlow>> {
+inputs![]
+}
+
+#[cfg(all(feature1 = "std1",feature2 = "std2",))]
+#[cfg(all(feature1 = "long textttttttttttttttttttttttttttttttttttttttt",feature2 = "long textttttttttttttttttttttttttttttttttttttttt",))]
+#[cfg(not(all(
+feature1 = "long textttttttttttttttttttttttttttttttttttttttt",
+feature2 = "long textttttttttttttttttttttttttttttttttttttttt",
+)))]
+type Os = NoSource;
+
+#[cfg(not(all(
+feature = "std",
+any(target_os = "linux",
+target_os = "android",
+)
+)))]
+#[cfg(not(all(
+feature = "std",
+any(target_os = "linuxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
+target_os = "android",
+)
+)))]
+#[cfg(not(all(
+any(
+target_os = "linux",
+target_os = "android",
+),
+feature = "std",
+)))]
+#[cfg(
+any(
+target_os = "linux",
+target_os = "android",
+)
+)]
+#[cfg(
+target_os = "linux",
+target_os = "android",
+)]
+#[cfg(not(all(
+target_os = "linux",
+target_os = "android",
+),
+feature = "std",
+))]    
+type Os = NoSource;
+
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows",
+)
+)
+)]
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows",
+),
+target_os = "unix",
+)
+)]
+type Os = NoSource;
+
+// Tests with no comma at the end of the last list items
+#[live_prop_test(
+precondition = "inputs.input_flows.len() == self.num_inputs()",
+postcondition = "result.len() == self.num_outputs()",
+postcondition = "output_times_valid(inputs, &result)"
+)]
+fn output_flows(
+&self,
+inputs: MachineObservedInputs,
+future: &Self::Future,
+) -> Inputs<Option<MaterialFlow>> {
+inputs![]
+}
+
+#[cfg(all(feature1 = "long textttttttttttttttttttttttttttttttttttttttt",feature2 = "long textttttttttttttttttttttttttttttttttttttttt"))]
+#[cfg(not(all(
+feature1 = "long textttttttttttttttttttttttttttttttttttttttt",
+feature2 = "long textttttttttttttttttttttttttttttttttttttttt"
+)))]
+type Os = NoSource;
+
+#[cfg(not(all(
+feature = "std",
+any(target_os = "linux",
+target_os = "android"
+)
+)))]
+#[cfg(not(all(
+any(
+target_os = "linux",
+target_os = "android"
+),
+feature = "std"
+)))]
+#[cfg(
+any(
+target_os = "linux",
+target_os = "android"
+)
+)]
+#[cfg(
+target_os = "linux",
+target_os = "android"
+)]
+type Os = NoSource;
+
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows"
+)
+)
+)]
+#[cfg(
+any(
+target_os = "linux",
+all(
+target_os = "android",
+target_os = "windows"
+),
+target_os = "unix"
+)
+)]
+type Os = NoSource;

tests/target/issue-3228/one.rs

@@ -0,0 +1,7 @@
+// rustfmt-version: One
+
+// Making sure trailing commas are not removed from attributes (no need for test source)
+#![cfg_attr(feature = "cargo-clippy", allow(clippy::inline_always,))]
+#![cfg_attr(feature = "cargo-clippy", allow(clippy::inline_always,),)]
+#![cfg_attr(feature = "cargo-clippy",)]
+#![allow(nline_always,)]