Assignment tracking for many-to-one assignments #181
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #181 +/- ##
==========================================
- Coverage 89.19% 89.11% -0.09%
==========================================
Files 55 55
Lines 9165 9176 +11
==========================================
+ Hits 8175 8177 +2
- Misses 825 832 +7
- Partials 165 167 +2 ☔ View full report in Codecov by Sentry. |
| @@ -749,3 +750,22 @@ func CheckGuardOnFullTrigger(trigger annotation.FullTrigger) annotation.FullTrig | |||
| } | |||
| return trigger | |||
| } | |||
|
|
|||
| // addAssignmentToConsumer updates the consumer with assignment entries for informative printing of errors | |||
| func addAssignmentToConsumer(lhs, rhs ast.Expr, pass *analysis.Pass, consumer annotation.ConsumingAnnotationTrigger) (err error) { | |||
There was a problem hiding this comment.
nit: we can drop the named return since it's not necessary :)
There was a problem hiding this comment.
Agreed. Done :)
| func addAssignmentToConsumer(lhs, rhs ast.Expr, pass *analysis.Pass, consumer annotation.ConsumingAnnotationTrigger) (err error) { | ||
| var lhsExprStr, rhsExprStr string | ||
| if lhsExprStr, err = asthelper.PrintExpr(lhs, pass, true /* isShortenExpr */); err != nil { | ||
| return err |
There was a problem hiding this comment.
Can we add more context to the error here? fmt.Errorf("adding assignment to consumer: %w", err)
There was a problem hiding this comment.
Good idea. Done!
| return err | ||
| } | ||
| if rhsExprStr, err = asthelper.PrintExpr(rhs, pass, true /* isShortenExpr */); err != nil { | ||
| return err |
| return err | ||
| } | ||
|
|
||
| consumer.AddAssignment(annotation.Assignment{ |
There was a problem hiding this comment.
Not something to be handled in this PR, but do you think we can actually move the logic (the asthelper.PrintExpr) inside the AddAssignment method? meaning make it take the AST nodes and do the expr shortening inside. I feel that would make the code simpler, but I may have missed some context.
There was a problem hiding this comment.
We could. However, that would mean passing the *analysis.Pass to ConsumeTrigger, which is not required by ConsumeTrigger otherwise. Maybe we can do this refactor later if that seems to be the better approach.
sonalmahajan15
force-pushed
the
sonalmahajan15/backprop-error-message-for-many-to-one-assignment
branch
from
17db0f6 to
d9aa057
Compare
sonalmahajan15
deleted the
sonalmahajan15/backprop-error-message-for-many-to-one-assignment
branch
This PR adds assignment tracking for many-to-one assignments for printing informative error messages, following suit of one-to-one assignment tracking (PR #87 ).