feat(#107): wrong-sprintf-arguments lint#206
Conversation
|
@maxonfjvipon please check |
| <xsl:variable name="decimal" select="sum(for $i in 1 to $length return (index-of(string-to-codepoints('0123456789ABCDEF'), string-to-codepoints(substring($hex-upper, $i, 1))) - 1) * xs:integer(math:pow(16, $length - $i)))"/> | ||
| <xsl:sequence select="$decimal"/> | ||
| </xsl:function> | ||
| <xsl:variable name="sprintf" select="//o[@base='.sprintf'][o[@base='.txt']/o[@base='QQ']]"/> |
There was a problem hiding this comment.
@h1alexbel QQ is a placeholder for Q.org.eolang, so it'll be //o[@base='.sprintf' and o[@base='.txt']/o[@base='.eolang']/o[@base='.org']/o[@base='Q']] or //o[@base='org.eolang.txt.sprintf']
| </xsl:function> | ||
| <xsl:variable name="sprintf" select="//o[@base='.sprintf'][o[@base='.txt']/o[@base='QQ']]"/> | ||
| <xsl:variable name="placeholder"> | ||
| <xsl:for-each select="tokenize($sprintf, '-')"> |
There was a problem hiding this comment.
@h1alexbel I don't really understand what you're tokenizing here, $sprintf is the element <o base='.sprintf>...`, or I'm wrong?
There was a problem hiding this comment.
@maxonfjvipon here, $sprintf will be a value from a text() node inside sprintf object:
<o base=".sprintf" line="4" pos="8">
<o base=".txt" line="4" pos="4">
<o base="QQ" line="4" pos="2"/>
</o>
<o base="string" line="5" pos="4">
48-65-6C-6C-6F-2C-20-25-73-21-20-59-6F-75-72-20-61-63-63-6F-75-6E-74-20-69-73-20-25-64-2E
</o>
<o base="tuple" line="6" pos="4">
<o base=".empty">
<o base="tuple"/>
</o>
<o base="name" line="6" pos="6"/>
</o>
</o>48-65-6C-6C-6F-2C-20-25-73-21-20-59-6F-75-72-20-61-63-63-6F-75-6E-74-20-69-73-20-25-64-2E will be selected as $sprintf here
There was a problem hiding this comment.
@h1alexbel what if the tuple (2nd argument of sprintf) would also contain a string or number?
sprintf
"Hello, %s"
* "Jeff"
In XMIR it would look like this:
<o base=".sprintf" line="4" pos="8">
<o base=".txt" line="4" pos="4">
<o base="QQ" line="4" pos="2"/>
</o>
<o base="string" line="5" pos="4">
48-65-6C-6C-6F-2C-20-25-73-21-20-59-6F-75-72-20-61-63-63-6F-75-6E-74-20-69-73-20-25-64-2E
</o>
<o base="tuple" line="6" pos="4">
<o base=".empty">
<o base="tuple"/>
</o>
<o base="string" line="5" pos="4">
48-65-6C-6C-6F
</o>
</o>
</o>What will the result of $sprintf variable in such case?
| <xsl:value-of select="codepoints-to-string(eo:hex-to-placeholder(.))"/> | ||
| </xsl:for-each> | ||
| </xsl:variable> | ||
| <xsl:variable name="declared" select="count(matches($placeholder, '%s')) + count(matches($placeholder, '%d'))"/> |
There was a problem hiding this comment.
@h1alexbel sprintf supports more formatters than just %d and %s
|
@maxonfjvipon updated. Take a look, please |
|
@maxonfjvipon reminder |
| </xsl:function> | ||
| <xsl:variable name="sprintf" select="//o[@base='.sprintf'][o[@base='.txt']/o[@base='QQ']]"/> | ||
| <xsl:variable name="placeholder"> | ||
| <xsl:for-each select="tokenize($sprintf, '-')"> |
There was a problem hiding this comment.
@h1alexbel what if the tuple (2nd argument of sprintf) would also contain a string or number?
sprintf
"Hello, %s"
* "Jeff"
In XMIR it would look like this:
<o base=".sprintf" line="4" pos="8">
<o base=".txt" line="4" pos="4">
<o base="QQ" line="4" pos="2"/>
</o>
<o base="string" line="5" pos="4">
48-65-6C-6C-6F-2C-20-25-73-21-20-59-6F-75-72-20-61-63-63-6F-75-6E-74-20-69-73-20-25-64-2E
</o>
<o base="tuple" line="6" pos="4">
<o base=".empty">
<o base="tuple"/>
</o>
<o base="string" line="5" pos="4">
48-65-6C-6C-6F
</o>
</o>
</o>What will the result of $sprintf variable in such case?
|
@maxonfjvipon thanks for the idea above, updated PR, take a look, please |
|
@maxonfjvipon reminder |
| [] > app | ||
| QQ.io.stdout > @ | ||
| org.eolang.txt.sprintf | ||
| "Hello, %s! Your account is %d." |
There was a problem hiding this comment.
@h1alexbel sprintf requires 2 mandatory argument, so if you want to provide empty tuple - just do add * as the second argument
| <xsl:variable name="used" select="count($tupled[not(@base='tuple')]/@base) + count(tokenize(substring($nested, 1, string-length($nested) - 1), '\s+'))"/> | ||
| <xsl:template match="/"> | ||
| <defects> | ||
| <xsl:if test="$sprintf-text != '' and $declared != $used"> |
There was a problem hiding this comment.
@h1alexbel what if there are not one sprintf in the file?
|
@maxonfjvipon updated. Take a look, please |
|
@maxonfjvipon reminder |
1 similar comment
|
@maxonfjvipon reminder |
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ | ||
| not.sprintf |
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ | ||
| not.sprintf |
| input: | | ||
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ |
| input: | | ||
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ |
| input: | | ||
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ |
| input: | | ||
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ |
| input: | | ||
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ |
| input: | | ||
| # App. | ||
| [] > app | ||
| QQ.io.stdout > @ |
|
@maxonfjvipon fixed indentation, please check again |
|
@yegor256 take a look, please |
| if (rule instanceof SpellingCheckRule) { | ||
| ((SpellingCheckRule) rule).addIgnoreTokens( | ||
| Arrays.asList("decoratee", "eolang") | ||
| Arrays.asList("decoratee", "eolang", "sprintf") |
There was a problem hiding this comment.
@h1alexbel better enquote the string inside the error message, to make it look like this:
According to the formatting template of the "sprintf" object, a tuple of 3 elements is expected as the second argument of it, while a tuple of 2 elements is provided
|
@h1alexbel excellent contribution, thanks! Could you please try this test too (must issue no warning): |
|
@yegor256 updated. Take a look, please |
|
@rultor merge |
|
@h1alexbel thanks! |
|
@maxonfjvipon Thank you for your review! We appreciate your contribution. Based on our team's policy, you've earned +11 points: +4 as the base reward, and +7 for the 26 comments you've made (capped at 8 points for comments). This aligns with our goal of encouraging thorough code reviews. Your current balance stands at +44. Keep up the great work in providing detailed feedback to your colleagues! |
|
@h1alexbel Hey there, great job on your contribution! 👍 You've earned +4 points, which is our base reward. We had to deduct 8 points because your code was a bit hefty (487 hits-of-code is over our 200 limit), and 6 points due to the high number of review comments (31). But don't worry, we added 14 points to keep you motivated! Your total balance is now +57. Remember, quality over quantity is key. Keep those contributions coming, and maybe aim for shorter, more focused code next time! |
5 participants
In this pull I've introduced new lint:
wrong-sprintf-arguments, that issueswarningdefect ifQQ.txt.sprintfobject hasdifferent number of passed arguments than number of its placeholder variables.
closes #107
History:
sprintf#107): decode ascii to stringsprintf#107): sprintf argssprintf#107): nested-argssprintf#107): casessprintf#107): passessprintf#107): more testssprintf#107): testssprintf#107): wrong-sprintf-arguments, fqnsprintf#107): clean for qulice