linter: add regression test for outer local used in object assert (#778)#872
Closed
bejaratommy wants to merge 20 commits intogoogle:masterfrom
Closed
linter: add regression test for outer local used in object assert (#778)#872bejaratommy wants to merge 20 commits intogoogle:masterfrom
bejaratommy wants to merge 20 commits intogoogle:masterfrom
Conversation
…date Bumps the go_modules group with 1 update in the / directory: [golang.org/x/crypto](https://github.com/golang/crypto). Bumps the go_modules group with 1 update in the /examples/bazel directory: [golang.org/x/crypto](https://github.com/golang/crypto). Updates `golang.org/x/crypto` from 0.36.0 to 0.45.0 - [Commits](golang/crypto@v0.36.0...v0.45.0) Updates `golang.org/x/crypto` from 0.36.0 to 0.45.0 - [Commits](golang/crypto@v0.36.0...v0.45.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.45.0 dependency-type: direct:production dependency-group: go_modules - dependency-name: golang.org/x/crypto dependency-version: 0.45.0 dependency-type: indirect dependency-group: go_modules ... Signed-off-by: dependabot[bot] <support@github.com>
…a76eca Also fix update_cpp_jsonnet.sh because the bazel/ directory was removed. Update test goldens (due to line shifts caused by standard library changes)
There were various bits of dead code and easily simplified things.
Fixes google#858. This is intended to match the restriction added in C++ Jsonnet in google/jsonnet#1217 and updated in google/jsonnet#1240
This doesn't make them fully correct, in particular directives sections (e.g., "%YAML 1.2" directive before a document start marker) are not handled correctly, but they weren't handled correctly before, either. It also doesn't recognise or try to do anything about document end markers (`...`). This fix does allow scalar documents to be on the same line as the document start tag which is valid per examples in the YAML spec, see for example https://yaml.org/spec/1.2.1/#id2760844 (YAML 1.2.1 spec, section 2.3 Scalars) It also matches the C++ jsonnet output for std.parseYaml("42\n---"), which is a stream of two documents, a scalar and then an empty document (where the empty document is interpreted as JSON null)
…-place Fixes google#768 Also the go-jsonnet version of google/jsonnet#365
For GitHub first party actions (actions from the github.com/actions organisation) we just use a major version tag. For actions from any other source we pin to an exact commit SHA1 (and put the version in a comment)
…ch64 wheels Build Linux aarch64 wheels by extending the build_wheels job in the workflow to also run on an `ubuntu-22.04-arm` runner. In order for this to work, the commit also extends the `install-go.sh` helper architecture aware.
builtinManifestJSONEx, builtinManifestYamlDoc and builtinManifestTomlEx
use native Go recursion that bypasses MaxStack. A self-referential
object like {a: $} causes unbounded memory allocation until OOM.
Add i.newCall() at the top of each recursive closure, matching the
pattern used by manifestJSON in interpreter.go.
Adds a test case demonstrating that a local variable defined in an outer scope (outside the object) is correctly recognized as used when it appears inside an object-level assert expression. Prevents regression of the fix introduced in google#723. Closes google#778
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
6ab4287 to
ab124ce
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Issue #778 reports that
jsonnet-lintincorrectly flags a local variable as unused when it is only referenced inside an object-levelassertexpression:The linter was incorrectly reporting:
Root Cause & Fix
The underlying fix was already introduced in #723 (commit e0c6a9e), which added traversal of
DesugaredObject.Assertsduring variable-usage analysis infindVariablesInObject. After that fix, object-level assert expressions are walked with the correct scope — including variables from outerlocalbindings — so occurrences are properly recorded.This PR
Adds a dedicated regression test case covering the exact scenario reported in #778: an outer (non-object-level)
localused exclusively inside an objectassert. The expected golden output is empty, confirming no spurious "unused variable" error is emitted.Closes #778