Ruby: add XML tables to dbscheme #10863
Conversation
There was a problem hiding this comment.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
| XmlParent getParent() { xmlChars(this, _, result, _, _, _) } | ||
|
|
||
| /** Holds if this character sequence is CDATA. */ | ||
| predicate isCDATA() { xmlChars(this, _, _, _, 1, _) } |
Check warning
Code scanning / CodeQL
Acronyms should be PascalCase/camelCase.
| description: <INSERT DESCRIPTION HERE> | ||
| compatibility: full|backwards|partial|breaking |
There was a problem hiding this comment.
You forgot to change this.
There was a problem hiding this comment.
Why did the PR checks pass?
There was a problem hiding this comment.
Ah indeed. I changed it, but I regenerated the files because I had forgotten about the numlines table.
There was a problem hiding this comment.
Why did the PR checks pass?
CI doesn't check the description or compatibility fields. It only checks the dbscheme and whether the upgrade queries can be compiled (there are none in this case). The "upgrade" is simply adding empty tables which can be achieved by doing nothing at all ;-)
| /** | ||
| * Provides classes and predicates for working with XML files and their content. |
There was a problem hiding this comment.
This seems like another candidate for a shared library pack, but that can be done separately.
There was a problem hiding this comment.
I actually copied the XML.qll files that the other languages share via identical-files.json . Unfortunately, the shared file contains an invalid (for us) import semmle.files.FileSystem statement. In addition the shared file has very many deprecated, renamed classes. So I cleaned it up.
Ideally we make this a proper shared library pack based on @erik-krogh's work on a shared Locations/Files library.
There was a problem hiding this comment.
Ideally we make this a proper shared library pack based on @erik-krogh's work on a shared Locations/Files library.
I think it will take a while before I got the CLI features I need to make that library. So don't wait on that.
| --language ruby --source-root "${{ github.workspace }}/repo" \ | ||
| "${{ runner.temp }}/database" | ||
| codeql database trace-command --threads 4 --index-traceless-dbs "${{ runner.temp }}/database" | ||
| codeql database index-files -l xml --include "**/*.xml.erb" "${{ runner.temp }}/database" |
There was a problem hiding this comment.
I am a bit surprised that our xml parser handles xml.erb files in the general case. Do you think there could be spurious syntax errors, or will it be guaranteed to work?
There was a problem hiding this comment.
That's a good point. I don't think it would work in the general case, but it might be alright for all the .xml.erb files in the five projects above.
There was a problem hiding this comment.
There will be syntax errors especially on <% token, but it mostly works. Note that this workflow is just for generating stats. Real XML files are pretty rare in Ruby projects, so this was the closest thing I could find.
There was a problem hiding this comment.
That's a good point. I don't think it would work in the general case, but it might be alright for all the .xml.erb files in the five projects above.
There are plenty of XML syntax errors in the xml.erb files in the 5 projects we currently use. However, these errors are non-fatal. The purpose is just to gather some stats about XML-like data. For that purpose it works fine.
We could, but I don't think it is very useful. Let's wait for a use-case first. It took me a while to find some repo with a non-trivial amount of xml-like data to gather stats, so I suspect that XML data is not very popular in Ruby code bases.. |
No description provided.