feat(#120): pos-bigger-than-line #195
Conversation
|
@maxonfjvipon please check |
| <xsl:output encoding="UTF-8" method="xml"/> | ||
| <xsl:template match="/"> | ||
| <defects> | ||
| <xsl:for-each select="//o[(@pos and @line) and (number(@pos)>number(@line))]"> |
There was a problem hiding this comment.
@h1alexbel please read the original ticket more carefully. @pos must be less that LEGTH of the line it belongs to. When we parse EO every object has its line and position at this line, for example here object foo would have position 6:
hello foo
But it can't have position 20 because the line length is 9
There was a problem hiding this comment.
@maxonfjvipon Ah, got it, thanks for the clarification, I will adjust this PR
|
@maxonfjvipon updated. Take a look, please |
| <defects> | ||
| <xsl:for-each select="//o"> | ||
| <xsl:variable name="line" select="number(@line)"/> | ||
| <xsl:variable name="length" select="string-length(normalize-space($lines[$line]))"/> |
There was a problem hiding this comment.
@h1alexbel I'm not sure if it's a right choice to take length from listing. For example, in JEO repository we create XMIR from Java bytecode which would look like binary one line text in listing. Maybe it would be better to find all the <o> with the same line, count and sum the lengths of their text() and compare with @pos?
@yegor256 WDYT?
There was a problem hiding this comment.
@maxonfjvipon Agree, I think it will be more correct way to calculate the length of the <o/> line
There was a problem hiding this comment.
@h1alexbel be careful because there may be a performance issue since for every <o> element you need to find all other elements with the same @line
|
@maxonfjvipon updated. Take a look, please. P.S. I'm not sure that is right way to calculate line length still |
|
@maxonfjvipon reminder |
|
@maxonfjvipon take a look, please |
|
@maxonfjvipon reminder |
| document: | | ||
| <program> | ||
| <objects> | ||
| <o line="2" pos="3" name="foo"> |
There was a problem hiding this comment.
@h1alexbel this example in EO will look like this:
[] ([] ([]) > xyz) > foo
Yes, it's legal in EO, the only part which is not allowed is most deepest abstract object. It can't contain data and it cant' be nameless
| document: | | ||
| <program> | ||
| <objects> | ||
| <o line="2" pos="20" name="foo"> |
There was a problem hiding this comment.
@h1alexbel as I mentioned here, this example in EO will look like this:
[] ([] ([]) > xyz) > foo
It almost legal in your case, BUT this EO line has more that 20 symbols which means no error should be caught
| <defects> | ||
| <xsl:for-each select="//o"> | ||
| <xsl:variable name="line" select="number(@line)"/> | ||
| <xsl:variable name="length" select="sum(//o[@line = $line and not(descendant::o)]/string-length(normalize-space(text())))"/> |
There was a problem hiding this comment.
@h1alexbel no, it still looks not correct to me.
Here's how I would try to implement it:
- take EO code and convert it to XMIR. You'll get "real" XMIR, not a toy one which you made up
- look at XMIR and EO code and try to make up with an algorithm of calculating line length
In general:
- if you have
@name- it EO it looks like this> foo, which isfoolength + 2 extra symbols - if you have
@base- it just@baselength - if it's long horizontal application like this
x (a b c) (y 1 2) (* (o 2) (q w e))you may see that that there are 10 braces and some amount of spaces without them, so they should be counted as well
Maybe there're some more patterns, I'm not sure, so the task is not such trivial as it seems to be
|
@maxonfjvipon thanks for ideas, I will adjust this PR |
In this pull I've introduced new lint:
pos-bigger-than-line, that issuescriticaldefect if@posis bigger than@lineattribute of the object in XMIR.closes #120