nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: bazelbuild/buildtools
Pull request #1192 (2023-08-31 13:44:28)

Compare load statements case-insensitively


Patch:
diff --git a/warn/warn_cosmetic.go b/warn/warn_cosmetic.go
index 7ee5f49..08ae8e7 100644
--- a/warn/warn_cosmetic.go
+++ b/warn/warn_cosmetic.go
@@ -120,8 +120,8 @@ func packageOnTopWarning(f *build.File) []*LinterFinding {
 			}
 			return []*LinterFinding{makeLinterFinding(rule.Call,
 				"Package declaration should be at the top of the file, after the load() statements, "+
-						"but before any call to a rule or a macro. "+
-						"package_group() and licenses() may be called before package().")}
+					"but before any call to a rule or a macro. "+
+					"package_group() and licenses() may be called before package().")}
 		}
 		seenRule = true
 	}
@@ -206,11 +206,17 @@ func comparePaths(path1, path2 string) bool {
 			return false
 		}
 		chunk2 := chunks2[i]
-		if chunk1 != chunk2 {
-			return chunk1 < chunk2
+		// Compare case-insensitively
+		chunk1Lower := strings.ToLower(chunk1)
+		chunk2Lower := strings.ToLower(chunk2)
+		if chunk1Lower != chunk2Lower {
+			return chunk1Lower < chunk2Lower
 		}
 	}
-	return true
+
+	// No case-insensitive difference detected. Likely the difference is just in
+	// the case of some symbols, compare case-sensitively for the determinism.
+	return path1 <= path2
 }
 
 // compareLoadLabels compares two module names


Test patch:
diff --git a/warn/warn_cosmetic_test.go b/warn/warn_cosmetic_test.go
index 43adf82..7286244 100644
--- a/warn/warn_cosmetic_test.go
+++ b/warn/warn_cosmetic_test.go
@@ -307,6 +307,25 @@ load("//foo:b.bzl", "b")`,
 		[]string{
 			":2: Load statement is out of its lexicographical order.",
 		}, scopeEverywhere)
+
+	checkFindingsAndFix(t, "out-of-order-load", `
+load("//Foo:aaa.bzl", "bar1")
+load("//Foo:Bbb.bzl", "bar2")
+load("//Foo:BBB.bzl", "bar3")
+load("//bar/Baz2:bar.bzl", "bar4")
+load("//bar/baz1:bar.bzl", "bar5")
+
+`, `
+load("//bar/baz1:bar.bzl", "bar5")
+load("//bar/Baz2:bar.bzl", "bar4")
+load("//Foo:aaa.bzl", "bar1")
+load("//Foo:BBB.bzl", "bar3")
+load("//Foo:Bbb.bzl", "bar2")`,
+		[]string{
+			":3: Load statement is out of its lexicographical order.",
+			":4: Load statement is out of its lexicographical order.",
+			":5: Load statement is out of its lexicographical order.",
+		}, scopeEverywhere)
 }
 
 func TestUnsortedDictItems(t *testing.T) {
Repository: bazelbuild/buildtools
Pull request #1202 (2023-09-28 17:58:20)

buildifier: fix uninitialised check when using list unpack assignment
Fixes https://github.com/bazelbuild/buildtools/issues/1201

As the test added in this commit shows, prior to this commit, the uninitialized check would incorrectly fail when using list unpack assignment, if the same variable is reused later.

This modifies `CollectLValues`, so that it supports list unpack assignments (in addition to tuple, and normal assignments).

Please let me know if there's anything that needs to be added/modified in this change, thanks! 🙏 

vladmos: Could you please update the branch? Usually I can do this myself just before merging, but now I likely don't have permissions to do so.
Gandem: @vladmos Done, thanks 🙇 !

Patch:
diff --git a/bzlenv/bzlenv.go b/bzlenv/bzlenv.go
index 43d94ce..40394af 100644
--- a/bzlenv/bzlenv.go
+++ b/bzlenv/bzlenv.go
@@ -148,6 +148,10 @@ func CollectLValues(node build.Expr) []*build.Ident {
 		for _, item := range node.List {
 			result = append(result, CollectLValues(item)...)
 		}
+	case *build.ListExpr:
+		for _, item := range node.List {
+			result = append(result, CollectLValues(item)...)
+		}
 	}
 	return result
 }


Test patch:
diff --git a/warn/warn_control_flow_test.go b/warn/warn_control_flow_test.go
index dc7da12..2c7418c 100644
--- a/warn/warn_control_flow_test.go
+++ b/warn/warn_control_flow_test.go
@@ -1281,6 +1281,15 @@ def foo():
 
   for x, y in z:
     bar(x)
+`,
+		[]string{},
+		scopeEverywhere)
+
+	checkFindings(t, "uninitialized", `
+def foo():
+  [x, y] = [1, 2]
+  x = 3
+  print(x)
 `,
 		[]string{},
 		scopeEverywhere)
Repository: bazelbuild/buildtools
Pull request #1211 (2023-10-16 16:30:56)

Fix package-on-top warning
Allow `licenses()` and `package_group()` statements appear before `package()` (like it's been implemented before).


Patch:
diff --git a/warn/warn_cosmetic.go b/warn/warn_cosmetic.go
index f618a52..9061619 100644
--- a/warn/warn_cosmetic.go
+++ b/warn/warn_cosmetic.go
@@ -102,7 +102,7 @@ func sameOriginLoadWarning(f *build.File) []*LinterFinding {
 	return findings
 }
 
-// packageOnTopWarning hoists package statements to the top after any comments / docstrings / load statments.
+// packageOnTopWarning hoists package statements to the top after any comments / docstrings / load statements.
 // If applied together with loadOnTopWarning and/or outOfOrderLoadWarning, it should be applied after them.
 // This is currently guaranteed by sorting the warning categories names before applying them:
 // "load-on-top" < "out-of-order-load" < "package-on-top"
@@ -120,7 +120,9 @@ func packageOnTopWarning(f *build.File) []*LinterFinding {
 		_, isString := stmt.(*build.StringExpr) // typically a docstring
 		_, isComment := stmt.(*build.CommentBlock)
 		_, isLoad := stmt.(*build.LoadStmt)
-		if isString || isComment || isLoad || stmt == nil {
+		_, isLicenses := edit.ExprToRule(stmt, "licenses")
+		_, isPackageGroup := edit.ExprToRule(stmt, "package_group")
+		if isString || isComment || isLoad || isLicenses || isPackageGroup || stmt == nil {
 			continue
 		}
 		rule, ok := edit.ExprToRule(stmt, "package")


Test patch:
diff --git a/warn/warn_cosmetic_test.go b/warn/warn_cosmetic_test.go
index a32bd60..0009ca6 100644
--- a/warn/warn_cosmetic_test.go
+++ b/warn/warn_cosmetic_test.go
@@ -155,6 +155,36 @@ package()
 foo(baz)`,
 		[]string{},
 		scopeDefault|scopeBzl|scopeBuild)
+
+	checkFindingsAndFix(t,
+		"package-on-top",
+		`
+# Some comments
+
+"""This is a docstring"""
+
+load(":foo.bzl", "foo")
+load(":bar.bzl", baz = "bar")
+
+package_group(name = "my_group")
+licenses(["my_license"])
+foo(baz)
+package()`,
+		`
+# Some comments
+
+"""This is a docstring"""
+
+load(":foo.bzl", "foo")
+load(":bar.bzl", baz = "bar")
+
+package_group(name = "my_group")
+licenses(["my_license"])
+
+package()
+foo(baz)`,
+		[]string{":11: Package declaration should be at the top of the file, after the load() statements, but before any call to a rule or a macro. package_group() and licenses() may be called before package()."},
+		scopeDefault|scopeBzl|scopeBuild)
 }
 
 func TestLoadOnTop(t *testing.T) {
Repository: bazelbuild/buildtools
Pull request #1357 (2025-04-26 21:18:32)

Bug fix. Correctly identify expressions using &=, ^=, >>=, and <<= as assignment statements when parsing BUILD files.
Correctly identify expressions using &=, ^=, >>=, <<= as assignment statements when parsing BUILD files.

The Starlark grammar reference (https://github.com/bazelbuild/starlark/blob/master/spec.md#grammar-reference) identifies expressions using all of these as assignment statements, but the parser doesn't include them in the list, probably an old oversight.

This fixes issue #1330.

pdoyle12: I've added tests in the warn_control_flow_test.go file. (I did validate that these correctly catch the specific problem mentioned in the linked bug.) It turns out there already are formatting tests for these cases in the golden file (052.in); if you'd like a more specific test as well, let me know.
vladmos: You're right, I thought only some of the assignment expressions were mentioned there (as binary expressions, but there's no difference for the formatter).

Patch:
diff --git a/build/parse.y b/build/parse.y
index 0eaf470..7b23b4c 100644
--- a/build/parse.y
+++ b/build/parse.y
@@ -1076,7 +1076,7 @@ func binary(x Expr, pos Position, op string, y Expr) Expr {
 	ystart, _ := y.Span()
 
 	switch op {
-	case "=", "+=", "-=", "*=", "/=", "//=", "%=", "|=":
+	case "=", "+=", "-=", "*=", "/=", "//=", "%=", "&=", "|=", "^=", "<<=", ">>=":
 		return &AssignExpr{
 			LHS:       x,
 			OpPos:     pos,
diff --git a/build/parse.y.go b/build/parse.y.go
index 11dec91..794b9ab 100644
--- a/build/parse.y.go
+++ b/build/parse.y.go
@@ -165,7 +165,7 @@ func binary(x Expr, pos Position, op string, y Expr) Expr {
 	ystart, _ := y.Span()
 
 	switch op {
-	case "=", "+=", "-=", "*=", "/=", "//=", "%=", "|=":
+	case "=", "+=", "-=", "*=", "/=", "//=", "%=", "&=", "|=", "^=", "<<=", ">>=":
 		return &AssignExpr{
 			LHS:       x,
 			OpPos:     pos,


Test patch:
diff --git a/warn/warn_control_flow_test.go b/warn/warn_control_flow_test.go
index 2c7418c..c80b389 100644
--- a/warn/warn_control_flow_test.go
+++ b/warn/warn_control_flow_test.go
@@ -352,7 +352,15 @@ foo != bar
 
 foo += bar
 bar -= bar
-
+bar *= bar
+bar /= bar
+bar //= bar
+bar %= bar
+bar &= bar
+bar |= bar
+bar ^= bar
+bar <<= bar
+bar >>= bar
 `,
 		[]string{":1:", ":3:", ":4:", ":5:", ":6:"},
 		scopeEverywhere)
Repository: googleapis/python-bigquery
Pull request #1919 (2024-05-15 18:43:00)

feat: add support for map target type in Parquet options
The map target type creates a schema without the added key_value repeated field.



Linchin: Thank you @sclmn for adding the new fields for us! We usually require 100% test coverage. Could you please also add unit tests for the new properties, similar to unit tests [here](https://github.com/googleapis/python-bigquery/blob/a429e8fd997a8850d15d434089869cf24e53c9e6/tests/unit/test_format_options.py#L39)? We also require the first commit and PR title to follow [conventional commits](https://github.com/googleapis/python-bigquery/pull/1919/checks?check_run_id=25017312294). Could you follow the instruction in the link to update your PR? If you need any help with these, feel free to let us know :)
conventional-commit-lint-gcf[bot]: <!-- probot comment [1356997]-->
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use `automerge` label. Good luck human!

 -- conventional-commit-lint bot
https://conventionalcommits.org/
sclmn: I updated the PR. PTAL.

Patch:
diff --git a/google/cloud/bigquery/format_options.py b/google/cloud/bigquery/format_options.py
index 1208565a..ad5591b1 100644
--- a/google/cloud/bigquery/format_options.py
+++ b/google/cloud/bigquery/format_options.py
@@ -105,6 +105,21 @@ class ParquetOptions:
     def enable_list_inference(self, value: bool) -> None:
         self._properties["enableListInference"] = value
 
+    @property
+    def map_target_type(self) -> str:
+        """Indicates whether to simplify the representation of parquet maps to only show keys and values."""
+
+        return self._properties.get("mapTargetType")
+
+    @map_target_type.setter
+    def map_target_type(self, value: str) -> None:
+        """Sets the map target type.
+
+        Args:
+          value: The map target type (eg ARRAY_OF_STRUCT).
+        """
+        self._properties["mapTargetType"] = value
+
     @classmethod
     def from_api_repr(cls, resource: Dict[str, bool]) -> "ParquetOptions":
         """Factory: construct an instance from a resource dict.


Test patch:
diff --git a/tests/unit/test_format_options.py b/tests/unit/test_format_options.py
index c8fecbfa..94a01570 100644
--- a/tests/unit/test_format_options.py
+++ b/tests/unit/test_format_options.py
@@ -54,11 +54,17 @@ class TestParquetOptions:
         )
         assert not config.enum_as_string
         assert config.enable_list_inference
+        assert config.map_target_type is None
 
     def test_to_api_repr(self):
         config = self._get_target_class()()
         config.enum_as_string = True
         config.enable_list_inference = False
+        config.map_target_type = "ARRAY_OF_STRUCT"
 
         result = config.to_api_repr()
-        assert result == {"enumAsString": True, "enableListInference": False}
+        assert result == {
+            "enumAsString": True,
+            "enableListInference": False,
+            "mapTargetType": "ARRAY_OF_STRUCT",
+        }
Repository: djc/askama
Pull request #880 (2023-10-12 10:24:16)

Allow macros to take `self` as an argument
Fixes the error ``no field `self` on type `&Templ` `` in the following example

```rust
#[derive(askama::Template)]
#[template(
    ext = "html",
    source = "
        {% macro m(x) %}
            {{ x.name }}
        {% endmacro %}

        {# this doesn't work #}
        {% call m(self) %}
    "
)]
struct Templ {
    name: String,
}

fn main() {
    println!(
        "{}",
        Templ {
            name: "Templ".into()
        }
    );
}
```


PizzasBear: A temporary fix is to wrap `self` in parentheses, e.g.

```rust
#[derive(askama::Template)]
#[template(
    ext = "html",
    source = "
        {% macro m(x) %}
            {{ x.name }}
        {% endmacro %}

        {# this does work #}
        {% call m((self)) %}
    "
)]
struct Templ {
    name: String,
}

fn main() {
    println!(
        "{}",
        Templ {
            name: "Templ".into()
        }
    );
}
```
PizzasBear: I'm pretty new to pull requests and this repo, so I don't know how to add a test...
djc: Basically add your example from the PR description in `testing/tests/macro.rs` and add a test function to make sure it has the expected output.
PizzasBear: I've added the test

Patch:
diff --git a/askama_derive/src/generator.rs b/askama_derive/src/generator.rs
index 76b775b..6f561f8 100644
--- a/askama_derive/src/generator.rs
+++ b/askama_derive/src/generator.rs
@@ -952,7 +952,7 @@ impl<'a> Generator<'a> {
                 // If `expr` is already a form of variable then
                 // don't reintroduce a new variable. This is
                 // to avoid moving non-copyable values.
-                Expr::Var(name) => {
+                &Expr::Var(name) if name != "self" => {
                     let var = self.locals.resolve_or_self(name);
                     self.locals.insert(arg, LocalMeta::with_ref(var));
                 }


Test patch:
diff --git a/testing/templates/macro-self-arg.html b/testing/templates/macro-self-arg.html
new file mode 100644
index 0000000..c7c6d5b
--- /dev/null
+++ b/testing/templates/macro-self-arg.html
@@ -0,0 +1,5 @@
+{%- macro my_s(slf) -%}
+    {{ slf.s }}
+{%- endmacro -%}
+
+{%- call my_s(self) -%}
diff --git a/testing/tests/macro.rs b/testing/tests/macro.rs
index 7e910a9..faada37 100644
--- a/testing/tests/macro.rs
+++ b/testing/tests/macro.rs
@@ -83,3 +83,15 @@ fn str_cmp() {
     let t = StrCmpTemplate;
     assert_eq!(t.render().unwrap(), "AfooBotherCneitherD");
 }
+
+#[derive(Template)]
+#[template(path = "macro-self-arg.html")]
+struct MacroSelfArgTemplate<'a> {
+    s: &'a str,
+}
+
+#[test]
+fn test_macro_self_arg() {
+    let t = MacroSelfArgTemplate { s: "foo" };
+    assert_eq!(t.render().unwrap(), "foo");
+}
Repository: djc/askama
Pull request #889 (2023-10-25 09:16:41)

Improve `elif` error
It's based on https://github.com/djc/askama/pull/887 (only the two last commits are from this PR).

When switching from another jinja-like framework, it's frequent to forget keywords like `elif`, so instead of just throwing a generic error, it also suggests `else if`.

GuillaumeGomez: Updated the message (and the first commits too).
Kijewski: This is great, thank you for working on making errors more readable!

`I` in `ErrorContext<I>` is always `&'a str`, isn't it? Maybe you could replace the generic argument with a simple lifetime, then?

Maybe `IResult<&'a str, T, ErrorContext<'a>>` could then be type aliased to

```rust
/// Parser result
PResult<'a, T = &'a str> = IResult<&'a str, T, ErrorContext<'a>>;
```
GuillaumeGomez: I don't know what the future of `askama` is going to be so I'd rather keep using `T` and not specialize it. However I can create a type alias if you want?
djc: Ha, just posted very similar ideas about a type alias in #887 before I saw the discussion here. Great minds...

I think we're quite unlikely to change the type from `&'a str` so I think specializing would make sense.
GuillaumeGomez: Do you want me to do it in this PR or should I open a new one?

Patch:
diff --git a/askama_parser/src/node.rs b/askama_parser/src/node.rs
index 93fca73..dc05f48 100644
--- a/askama_parser/src/node.rs
+++ b/askama_parser/src/node.rs
@@ -299,7 +299,15 @@ impl<'a> Cond<'a> {
         let mut p = tuple((
             |i| s.tag_block_start(i),
             opt(Whitespace::parse),
-            ws(keyword("else")),
+            ws(alt((keyword("else"), |i| {
+                let _ = keyword("elif")(i)?;
+                Err(nom::Err::Failure(ErrorContext {
+                    input: i,
+                    message: Some(Cow::Borrowed(
+                        "unknown `elif` keyword; did you mean `else if`?",
+                    )),
+                }))
+            }))),
             cut(tuple((
                 opt(|i| CondTest::parse(i, s)),
                 opt(Whitespace::parse),


Test patch:
diff --git a/testing/tests/ui/elif.rs b/testing/tests/ui/elif.rs
new file mode 100644
index 0000000..650f292
--- /dev/null
+++ b/testing/tests/ui/elif.rs
@@ -0,0 +1,8 @@
+use askama::Template;
+
+#[derive(Template)]
+#[template(source = "{% if true %}{% elif false %}{% endif %}", ext = "html")]
+struct UnknownElif;
+
+fn main() {
+}
diff --git a/testing/tests/ui/elif.stderr b/testing/tests/ui/elif.stderr
new file mode 100644
index 0000000..0d5de47
--- /dev/null
+++ b/testing/tests/ui/elif.stderr
@@ -0,0 +1,9 @@
+error: unknown `elif` keyword; did you mean `else if`?
+       problems parsing template source at row 1, column 16 near:
+       "elif false %}{% endif %}"
+ --> tests/ui/elif.rs:3:10
+  |
+3 | #[derive(Template)]
+  |          ^^^^^^^^
+  |
+  = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)
Repository: djc/askama
Pull request #928 (2023-12-06 16:22:36)

Allow trailing comma in macro definition and call
Quite convenient when arguments are on different lines.

(First commit comes from https://github.com/djc/askama/pull/925, it will prevent to have a merge conflict once the first one will be merged)


Patch:
diff --git a/askama_parser/src/expr.rs b/askama_parser/src/expr.rs
index 2928b57..65a7835 100644
--- a/askama_parser/src/expr.rs
+++ b/askama_parser/src/expr.rs
@@ -119,7 +119,7 @@ impl<'a> Expr<'a> {
                         }
                     }),
                 ),
-                char(')'),
+                tuple((opt(ws(char(','))), char(')'))),
             )),
         )(i)
     }
diff --git a/askama_parser/src/node.rs b/askama_parser/src/node.rs
index 27d4adc..c2f7b39 100644
--- a/askama_parser/src/node.rs
+++ b/askama_parser/src/node.rs
@@ -493,7 +493,7 @@ impl<'a> Macro<'a> {
             delimited(
                 ws(char('(')),
                 separated_list0(char(','), ws(identifier)),
-                ws(char(')')),
+                tuple((opt(ws(char(','))), char(')'))),
             )(i)
         }
 


Test patch:
diff --git a/testing/tests/macro.rs b/testing/tests/macro.rs
index 9b3c63d..3d027d2 100644
--- a/testing/tests/macro.rs
+++ b/testing/tests/macro.rs
@@ -140,3 +140,33 @@ struct OnlyNamedArgument;
 fn test_only_named_argument() {
     assert_eq!(OnlyNamedArgument.render().unwrap(), "hi");
 }
+
+// Check for trailing commas.
+#[derive(Template)]
+#[template(
+    source = r#"{% macro button(label , ) %}
+{{- label -}}
+{% endmacro %}
+{%- macro button2(label ,) %}
+{% endmacro %}
+{%- macro button3(label,) %}
+{% endmacro %}
+{%- macro button4(label, ) %}
+{% endmacro %}
+{%- macro button5(label ) %}
+{% endmacro %}
+
+{%- call button(label="hi" , ) -%}
+{%- call button(label="hi" ,) -%}
+{%- call button(label="hi",) -%}
+{%- call button(label="hi", ) -%}
+{%- call button(label="hi" ) -%}
+"#,
+    ext = "html"
+)]
+struct TrailingComma;
+
+#[test]
+fn test_trailing_comma() {
+    assert_eq!(TrailingComma.render().unwrap(), "hihihihihi");
+}
Repository: djc/askama
Pull request #941 (2024-01-09 16:42:35)

Make `String` compatible with the markdown filter.
Hello, I recently decided to switch from Tera to Askama, and I'm enjoying it so far. 

Yesterday, I stumbled upon issue #719 and decided to see if I can fix the issue with the markdown filter, so I don't have to include Comrak in my own crate as a workaround. 
What I found out is that the issue with using `String` is that the filter moves the value out of the struct. This is fine for `&str` considering `&str` is already a reference. 
```rs
    /// Code generated by Template derive macro  
    fn render_into(&self, writer: &mut (impl ::std::fmt::Write + ?Sized)) -> ::askama::Result<()> {
    ...
            expr0 = &::askama::filters::markdown(::askama::Html, self.content, ::core::option::Option::None)?,
            //                                                   ^^^^^^^^^^^^ 
            //                                              Value gets moved out by filter 
    ...
```

Making the generated code borrow the value did not create any problems in tests.

This PR also adds a test for passing `String`s to the markdown filter and simplifies the function
signature for the filter to accept `&str`. 


GuillaumeGomez: Ok, now please run `cargo fmt` and then [squash your commits](https://blog.guillaume-gomez.fr/Git-tips/1/2).
djc: @GuillaumeGomez if there is a single conceptual change we don't need the submitter to squash -- we can just "Squash and merge" instead.
GuillaumeGomez: Ah true. I keep forgetting about it.

Patch:
diff --git a/askama_derive/src/generator.rs b/askama_derive/src/generator.rs
index df15bdc..760a1f6 100644
--- a/askama_derive/src/generator.rs
+++ b/askama_derive/src/generator.rs
@@ -1200,7 +1200,7 @@ impl<'a> Generator<'a> {
         };
 
         buf.write(&format!(
-            "::askama::filters::markdown({}, ",
+            "::askama::filters::markdown({}, &",
             self.input.escaper
         ));
         self.visit_expr(buf, md)?;


Test patch:
diff --git a/testing/tests/markdown.rs b/testing/tests/markdown.rs
index 75163c9..04c9ec9 100644
--- a/testing/tests/markdown.rs
+++ b/testing/tests/markdown.rs
@@ -70,3 +70,21 @@ before\
 after",
     );
 }
+
+#[derive(Template)]
+#[template(source = "{{content|markdown}}", ext = "html")]
+struct MarkdownStringTemplate {
+    content: String,
+}
+
+// Tests if the markdown filter accepts String
+#[test]
+fn test_markdown_owned_string() {
+    let template = MarkdownStringTemplate {
+        content: "The markdown filter _indeed_ works with __String__".into(),
+    };
+    assert_eq!(
+        template.render().unwrap(),
+        "<p>The markdown filter <em>indeed</em> works with <strong>String</strong></p>\n"
+    )
+}