Skip to content

Fixes problem with symbols as single expression inside if clause - #300

Merged
averissimo merged 4 commits into
mainfrom
299-fix_dependency
Oct 14, 2026
Merged

averissimo merged 4 commits into
mainfrom
299-fix_dependency

Conversation

@averissimo

Copy link
Copy Markdown
Contributor

Copilot AI balanced review requested due to automatic review settings October 9, 2026 16:13
@averissimo averissimo added the core label Oct 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused guard resolves the reported failure and is covered by a representative regression test.

1 open finding
What changed in this PR

Fixes symbol-only expressions inside if blocks so qenv dependency extraction no longer errors.

Changes:

  • Guards symbol expressions before AST subsetting.
  • Adds regression coverage for issue #299.
  • Documents the bug fix.
File Description
R/​utils-get_code_dependency.R Safely handles symbol expressions.
tests/​testthat/​test-qenv_within.R Adds regression coverage.
NEWS.md Records the fix.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread NEWS.md Outdated
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Unit Tests Summary

  1 files   14 suites   5s ⏱️
190 tests 186 ✅ 4 💤 0 ❌
274 runs  270 ✅ 4 💤 0 ❌

Results for commit 7115c7e.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
qenv_within 👶 $+0.02$ handles_name_expression_inside_if_clause

Results for commit a1aac7f

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------
R/qenv-c.R                          55       0  100.00%
R/qenv-class.R                      13       0  100.00%
R/qenv-concat.R                      7       0  100.00%
R/qenv-constructor.R                 1       0  100.00%
R/qenv-errors.R                      4       4  0.00%    6-9
R/qenv-eval_code.R                  63       1  98.41%   40
R/qenv-extract.R                    30       0  100.00%
R/qenv-get_code.R                   24       0  100.00%
R/qenv-get_env.R                     3       1  66.67%   27
R/qenv-get_messages.r                5       0  100.00%
R/qenv-get_outputs.R                 6       0  100.00%
R/qenv-get_var.R                    13       1  92.31%   13
R/qenv-get_warnings.R                5       0  100.00%
R/qenv-join.R                        1       1  0.00%    13
R/qenv-length.R                      2       1  50.00%   2
R/qenv-show.R                       29      29  0.00%    19-50
R/qenv-within.R                      8       0  100.00%
R/utils-get_code_dependency.R      258       3  98.84%   160, 258, 326
R/utils.R                           42       0  100.00%
TOTAL                              569      41  92.79%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: 7115c7e

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: André Veríssimo <211358+averissimo@users.noreply.github.com>

@wangben718 wangben718 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for working on this issue! I checked a few more cases (nested braces, for loops, constants, a symbol followed by reassigning the same object) and they all work.

Comment thread R/utils-get_code_dependency.R Outdated
Signed-off-by: André Veríssimo <211358+averissimo@users.noreply.github.com>
@averissimo
averissimo merged commit 6349f5c into main Oct 14, 2026
29 checks passed
@averissimo
averissimo deleted the 299-fix_dependency branch October 14, 2026 21:48
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Bug on code_dependency with expression only containing symbol

3 participants