Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Dynamic languages: Deprecate models-as-data CSV interface #16181

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Dynamic languages: Deprecate models-as-data CSV interface #16181

merged 5 commits into from
Apr 16, 2024

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Apr 11, 2024

No description provided.

@github-actions github-actions bot added the Ruby label Apr 11, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 11, 2024
View reviewed changes
ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll
}

private class DeprecationAdapterImpl extends DeprecationAdapter {
deprecated override predicate sourceModel(string type, string path, string kind) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll
)
}

deprecated override predicate sinkModel(string type, string path, string kind) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll
)
}

deprecated override predicate summaryModel(

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll

private predicate typeModel(string row) { any(TypeModelCsv s).row(inversePad(row)) }
deprecated override predicate typeModel(string type1, string type2, string path) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll

private predicate typeVariableModel(string row) { any(TypeVariableModelCsv s).row(inversePad(row)) }
deprecated override predicate typeVariableModel(string name, string path) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
@hvitved hvitved force-pushed the dynamic/deprecate-csv-models branch from 6467ddd to fdde1c7 Compare April 11, 2024 09:46
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 11, 2024
View reviewed changes
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll
}

private class DeprecationAdapterImpl extends DeprecationAdapter {
deprecated override predicate sourceModel(string type, string path, string kind) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll
)
}

deprecated override predicate sinkModel(string type, string path, string kind) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll
)
}

deprecated override predicate summaryModel(

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll

private predicate typeModel(string row) { any(TypeModelCsv s).row(inversePad(row)) }
deprecated override predicate typeModel(string type1, string type2, string path) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll

private predicate typeVariableModel(string row) { any(TypeVariableModelCsv s).row(inversePad(row)) }
deprecated override predicate typeVariableModel(string name, string path) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll
}

private class DeprecationAdapterImpl extends DeprecationAdapter {
deprecated override predicate sourceModel(string type, string path, string kind) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll
)
}

deprecated override predicate sinkModel(string type, string path, string kind) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll
)
}

deprecated override predicate summaryModel(

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll

private predicate typeModel(string row) { any(TypeModelCsv s).row(inversePad(row)) }
deprecated override predicate typeModel(string type1, string type2, string path) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll

private predicate typeVariableModel(string row) { any(TypeVariableModelCsv s).row(inversePad(row)) }
deprecated override predicate typeVariableModel(string name, string path) {

Check warning

Code scanning / CodeQL

Inconsistent deprecation Warning

This deprecated predicate overrides
a non-deprecated predicate
Loading
. Consider deprecating both.
@hvitved hvitved force-pushed the dynamic/deprecate-csv-models branch 3 times, most recently from e197557 to 0d5591f Compare April 11, 2024 11:45
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Apr 11, 2024
@hvitved hvitved marked this pull request as ready for review April 11, 2024 17:34
@hvitved hvitved requested review from a team as code owners April 11, 2024 17:34
yoff
yoff previously approved these changes Apr 12, 2024
View reviewed changes
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

Python 👍

Can we really get away with no change note, though? If people have models in CSV-files, they would need to convert them, right?

asgerf
asgerf previously approved these changes Apr 12, 2024
View reviewed changes
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

LGTM. I think the change note can be omitted, but perhaps the deprecated classes can have a QLDoc comment about how the contents should be migrated to a data extension instead. And in the case of TypeModelCsv, the TypeModel class is also an alternative.

javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll
@@ -134,15 +134,15 @@ module ModelInput {
* `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps,
* respectively.
*/
abstract predicate row(string row);
abstract deprecated predicate row(string row);
}

/**
* A unit class for adding additional type model rows.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* A unit class for adding additional type model rows.
* DEPRECATED. Use a data extension or the `TypeModel` class instead.
*
* A unit class for adding additional type model rows.

@hvitved hvitved dismissed stale reviews from asgerf and yoff via f04431b April 12, 2024 11:14
@hvitved hvitved force-pushed the dynamic/deprecate-csv-models branch from 0d5591f to f04431b Compare April 12, 2024 11:14
@hvitved hvitved force-pushed the dynamic/deprecate-csv-models branch from f04431b to e7dc120 Compare April 12, 2024 11:40
yoff
yoff approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

LGTM

@hvitved hvitved merged commit ff55ed8 into github:main Apr 16, 2024
26 checks passed
@hvitved hvitved deleted the dynamic/deprecate-csv-models branch April 16, 2024 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asgerf asgerf asgerf approved these changes

@yoff yoff yoff approved these changes

Assignees
No one assigned
Labels
JS no-change-note-required This PR does not need a change note Python Ruby
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hvitved @asgerf @yoff