Violation location in UnitTest

[Joris Dauphin]

In unitTest, we have to write the location of the violation which is error prone and not really readable.

I suggest something like:

-- 8< --

#define TAG_START "TAG_START"

#define TAG_END "TAG_END"

void ComputeLocalisation(const std::string& s, size_t offset,

                            int* line, int* column)

{

    int c = 1;

    int l = 1;

    for (size_t i = 0; i != offset; ++i) {

        if (s[i] == '\n') {

            c = 1;

            ++l;

        } else {

            ++c;

        }

    }

    *line = l;

    *column = c;

}

void testRuleOnCXXCodeWithViolation(RuleBase* rule, std::string code,

                        const std::string& message = "")

{

    const size_t startOffset = code.find(TAG_START);

    assert(startOffset != std::string::npos);

    code.erase(startOffset, strlen(TAG_START));

    const size_t endOffset = code.find(TAG_END, startOffset);

    assert(endOffset != std::string::npos);

    code.erase(endOffset, strlen(TAG_END));

    int startLine;

    int startColumn;

    int endLine;

    int endColumn;

    ComputeLocalisation(code, startOffset, &startLine, &startColumn);

    ComputeLocalisation(code, endOffset, &endLine, &endColumn);

    testRuleOnCXXCode(rule, code, 0, startLine, startColumn, endLine, endColumn, message);

}

------- >8 -------

And so the TestCode will be something like

-- 8< --

    testRuleOnCXXCodeWithViolation(new DeleteNullptrIsSafeRule(),

        "void m(char* pointer) { " TAG_START "if (pointer) delete " TAG_END "pointer; }");

-- >8 --

instead of

-- 8< --

    testRuleOnCXXCode(new DeleteNullptrIsSafeRule(),

    //                            1               2                3

    //            123456789012345678901234567890123456

                 "void m(char* pointer) { if (pointer) delete pointer; }", 0, 1, 19, 1, 33);

-- >8 --

I know, TAG_END seems miss placed, but this is really visible in the 1st example,

not in the second (and worst without the comment).

clang seems to have a bug with ifStmt->getEndLoc() (or with the delete Stmt, not investigated).

[Longyi Qi]

Hi Joris,

Thanks for the idea.

First of all, I think it's a good add-on to existing ways of writing tests. Certain tests might be easier by marking open/close tags; and others might be simpler by using the existing approaches. Especially, my understand right now is, this tag approach only works for code that has only one issue, or only the first one can be tagged. Correct me if I am wrong.

I am a little bit worried about where to put the tags, especially the close tags. Sometimes, the close tag has to be placed in a weird location. Is there a way to easily tell the offset of the close tag when rule authors place them by mistake?

In addition, TAG_START, TAG_END... there gonna be better names... :)

Thanks,

Longyi

--
You received this message because you are subscribed to the Google Groups "OCLint Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to oclint-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Joris Dauphin]

On Friday, September 27, 2013 3:14:28 AM UTC+2, Longyi Qi wrote:

Hi Joris,

Thanks for the idea.

First of all, I think it's a good add-on to existing ways of writing tests. Certain tests might be easier by marking open/close tags; and others might be simpler by using the existing approaches. Especially, my understand right now is, this tag approach only works for code that has only one issue, or only the first one can be tagged. Correct me if I am wrong.

No, it could be extended to several violations (as long as they don't interleave I would say).

I am a little bit worried about where to put the tags, especially the close tags. Sometimes, the close tag has to be placed in a weird location. Is there a way to easily tell the offset of the close tag when rule authors place them by mistake?

when a tag is misplaced, failed test reports the wanted offset and the current offset, so we have to move the tag from the difference in these cases.

 

In addition, TAG_START, TAG_END... there gonna be better names... :)

I fully agree.

 

[Joris Dauphin]

WIP: https://github.com/Jarod42/oclint.git branch:TestWithTagLocation

[Joris Dauphin]

Merged in master ^_^

So now, you may use VIOLATION_START, LOC_START, ... to specify locations of violations in unitTests.

[Longyi Qi]

Thank you so much for your hard work, Joris! I will look into your other pull requests. 

On Sat, Dec 7, 2013 at 10:48 AM, Joris Dauphin <joris....@gmail.com> wrote:

Merged in master ^_^

So now, you may use VIOLATION_START, LOC_START, ... to specify locations of violations in unitTests.

--