Skip to content

Conversation

aschackmull
Copy link
Contributor

This ensures that we show the toString and thus the accesspath in all contexts, in particular lgtm.com.

jbj
jbj previously approved these changes Sep 5, 2019
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@yh-semmle yh-semmle left a comment

Choose a reason for hiding this comment

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

Expected test output still needs updating.

@aschackmull
Copy link
Contributor Author

I've included a commit to fix the toString of an empty accesspath, which was a postponed change from #1797.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

See comment above.

@aschackmull
Copy link
Contributor Author

This means we should be able to get rid of the direct source-to-sink step

Possibly, yes. But I think that's best left to a separate PR. This is about getting the right toString to lgtm.com.

@aschackmull aschackmull added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Sep 12, 2019
@hvitved
Copy link
Contributor

hvitved commented Sep 12, 2019

Possibly, yes. But I think that's best left to a separate PR. This is about getting the right toString to lgtm.com.

OK, I would have thought the two would go together; the sink-to-source step was basically a workaround needed because of the missing nodes predicate.

@aschackmull
Copy link
Contributor Author

Language-Tests/Java is btw. not expected to succeed here, but it is green on the internal PR.

@hvitved hvitved merged commit f5cae9b into github:master Sep 13, 2019
@aschackmull aschackmull deleted the java/pathgraph-nodes branch September 13, 2019 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR Java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants