-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
C#: Add MaD support for Attribute.Getter
and Attribute.Setter
.
#17459
Conversation
cbd8340
to
3130df7
Compare
3130df7
to
3c911b2
Compare
…for use in external models.
3c911b2
to
bdc0084
Compare
DCA looks good! |
@@ -289,7 +288,7 @@ module ModelValidation { | |||
not signature.regexpMatch("|\\([a-zA-Z0-9_<>\\.\\+\\*,\\[\\]]*\\)") and | |||
result = "Dubious signature \"" + signature + "\" in " + pred + " model." | |||
or | |||
not ext.regexpMatch("|Attribute") and | |||
not ext = ["", "Attribute", "Attribute.Getter", "Attribute.Setter"] and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should mention these new extensions in the comment at the top of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
not result instanceof Property and | ||
not result instanceof Indexer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we be backwards compatible if we removed these two lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I think we should take a conscious decision and break the backwards compatibility - as it was quite surprising that tagging a property meant that it worked for setters.
Also, the public documentation doesn't contain the explanation on, how to use the ext
field, so it is most likely not heavily used (yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In this PR we added support for
Attribute.Getter
andAttribute.Setter
as a part of the models as data description in theext
field of a model.Assume we have the following declarations.
If we add the following source model
- ["N", "SourceAttribute", false, "", "", "Attribute.Getter", "ReturnValue", "local", "manual"]
Then
TaggedSrcPropertyGetter
is considered alocal
source of taint.