Skip to content

rename all occurrences of XML to Xml#10073

Merged
erik-krogh merged 4 commits into
github:mainfrom
erik-krogh:XMLXml
Aug 22, 2022
Merged

rename all occurrences of XML to Xml#10073
erik-krogh merged 4 commits into
github:mainfrom
erik-krogh:XMLXml

Conversation

@erik-krogh
Copy link
Copy Markdown
Contributor

@erik-krogh erik-krogh commented Aug 16, 2022

Acronyms should use normal PascalCase/camelCase.

written automatically using my patching utility

The CI here is failing, and that is intended. Look at the CI checks in the internal PR (see the first backref below).

I've also deleted a dead method in java/kotlin-extractor/src/main/kotlin/utils/List.kt, because that method being dead caused the internal CI to fail.

This PR has to be merged at the same time as the internal PR.

@smowton
Copy link
Copy Markdown
Contributor

smowton commented Aug 17, 2022

AFAIK generally we would keep the old name for public symbols and deprecate it to avoid a breaking change for 3rd-party queries? Given that, maybe it's best to restrict the rewrite to private symbols?

@erik-krogh
Copy link
Copy Markdown
Contributor Author

AFAIK generally we would keep the old name for public symbols and deprecate it to avoid a breaking change for 3rd-party queries? Given that, maybe it's best to restrict the rewrite to private symbols?

I've left a deprecated alias with the old name for every rename, isn't that good enough?
People will get a deprecation warning for at least a year before we can delete the deprecated name.

@smowton
Copy link
Copy Markdown
Contributor

smowton commented Aug 17, 2022

Oh sorry I somehow overlooked that, ignore me.

@erik-krogh erik-krogh force-pushed the XMLXml branch 3 times, most recently from dc2c568 to 7ef4172 Compare August 18, 2022 17:41
@erik-krogh erik-krogh added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Aug 19, 2022
@erik-krogh erik-krogh marked this pull request as ready for review August 19, 2022 13:16
@erik-krogh erik-krogh requested review from a team as code owners August 19, 2022 13:16
michaelnebel
michaelnebel previously approved these changes Aug 19, 2022
Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

C# 👍

tausbn
tausbn previously approved these changes Aug 19, 2022
Copy link
Copy Markdown
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

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

LGTM Lgtm! 👍

Comment thread cpp/ql/lib/semmle/code/cpp/XML.qll Outdated
@erik-krogh erik-krogh dismissed stale reviews from tausbn and michaelnebel via 108a0bb August 19, 2022 13:41
jketema
jketema previously approved these changes Aug 19, 2022
Copy link
Copy Markdown
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

C++ 👍

* ```
*/
class XMLDTD extends XMLLocatable, @xmldtd {
class XmlDTD extends XmlLocatable, @xmldtd {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in XmlDTD should be PascalCase/camelCase
* ```
*/
class XMLDTD extends XMLLocatable, @xmldtd {
class XmlDTD extends XmlLocatable, @xmldtd {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in XmlDTD should be PascalCase/camelCase
* ```
*/
class XMLDTD extends XMLLocatable, @xmldtd {
class XmlDTD extends XmlLocatable, @xmldtd {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in XmlDTD should be PascalCase/camelCase
* ```
*/
class XMLDTD extends XMLLocatable, @xmldtd {
class XmlDTD extends XmlLocatable, @xmldtd {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in XmlDTD should be PascalCase/camelCase
* ```
*/
class XMLDTD extends XMLLocatable, @xmldtd {
class XmlDTD extends XmlLocatable, @xmldtd {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in XmlDTD should be PascalCase/camelCase

/** Gets a DTD associated with this XML file. */
XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) }
XmlDTD getADTD() { xmlDTDs(result, _, _, _, this) }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in getADTD should be PascalCase/camelCase

/** Gets a DTD associated with this XML file. */
XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) }
XmlDTD getADTD() { xmlDTDs(result, _, _, _, this) }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in getADTD should be PascalCase/camelCase

/** Gets a DTD associated with this XML file. */
XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) }
XmlDTD getADTD() { xmlDTDs(result, _, _, _, this) }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in getADTD should be PascalCase/camelCase

/** Gets a DTD associated with this XML file. */
XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) }
XmlDTD getADTD() { xmlDTDs(result, _, _, _, this) }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in getADTD should be PascalCase/camelCase

/** Gets a DTD associated with this XML file. */
XMLDTD getADTD() { xmlDTDs(result, _, _, _, this) }
XmlDTD getADTD() { xmlDTDs(result, _, _, _, this) }

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase.

Acronyms in getADTD should be PascalCase/camelCase
@erik-krogh
Copy link
Copy Markdown
Contributor Author

QL-for-QL is complaining that the DTD in XmlDTD is not PascalCase.
I will fix that, but in a later PR.

kaeluka
kaeluka previously approved these changes Aug 22, 2022
Copy link
Copy Markdown

@kaeluka kaeluka left a comment

Choose a reason for hiding this comment

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

Javascript: 👍, with an optional comment

}

/** DEPRECATED: Alias for XmlDTD */
deprecated class XMLDTD = XmlDTD;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

while we're at it, can we name this one XmlDtd?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I going to rename more things in a later PR.
I'll include DTD in that one.

Copy link
Copy Markdown
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Java 👍

@erik-krogh erik-krogh merged commit eadd85b into github:main Aug 22, 2022
Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

C# Lgtm

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 documentation Java JS Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants