Rust: Never skip assignment LHS in data flow

[hvitved]

Always including the left-hand side of an assignment in data flow path graphs make it easier to follow flow. The same goes for scrutinee and patterns in match expressions.

Copilot wasn't able to review any files in this pull request.

Files not reviewed (7)

• rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll: Language not supported
• rust/ql/test/library-tests/dataflow/barrier/inline-flow.expected: Language not supported
• rust/ql/test/library-tests/dataflow/closures/inline-flow.expected: Language not supported
• rust/ql/test/library-tests/dataflow/global/inline-flow.expected: Language not supported
• rust/ql/test/library-tests/dataflow/local/inline-flow.expected: Language not supported
• rust/ql/test/library-tests/dataflow/models/models.expected: Language not supported
• rust/ql/test/library-tests/dataflow/taint/inline-taint-flow.expected: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

[paldepind]

This seems really nice!

Where these nodes hidden due to some defaults in the data flow library? They where not covered by our nodeIsHidden as far as I can tell.

[paldepind]

Suggested change

	node.getCfgNode() = any(LetStmtCfgNode s).getPat()
	or
	node.getCfgNode() = any(AssignmentExprCfgNode a).getLhs()
	node.getCfgNode() = [any(LetStmtCfgNode s).getPat(), any(AssignmentExprCfgNode a).getLhs()]

[hvitved]

Making this change results in

Could not determine type of set literal. For set literals, the type must be one of the types of the elements, but neither CfgNodes::MakeCfgNodes<Locations::Location, CfgNodes::CfgNodesInput>::Nodes::PatCfgNode nor CfgNodes::MakeCfgNodes<Locations::Location, CfgNodes::CfgNodesInput>::Nodes::ExprCfgNode is a supertype of the other.

Which means one has to instead write

node.getCfgNode() =
      [any(LetStmtCfgNode s).getPat().(CfgNode), any(AssignmentExprCfgNode a).getLhs().(CfgNode)]

and then I prefer the above.

[paldepind]

Ah, I see :)

[paldepind]

Suggested change

	node.asExpr() = match.getScrutinee() or
	node.asExpr() = match.getArmPat(_)
	node.asExpr() = [match.getScrutinee(), match.getArmPat(_)]

[hvitved]

Same.

[hvitved]

Where these nodes hidden due to some defaults in the data flow library? They where not covered by our nodeIsHidden as far as I can tell.

Nodes may be hidden because the data flow library compresses long local flow paths using a big-step relation. The predicate neverSkipInPathGraph tells the data flow library to never skip over certain nodes.

[paldepind]

Nodes may be hidden because the data flow library compresses long local flow paths using a big-step relation. The predicate neverSkipInPathGraph tells the data flow library to never skip over certain nodes.

I see. Thanks 🙏