marko10_000
/
openclonk
forked from Mirrors/openclonk
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Warn on assignments where an expression is expected 

This introduces a new diagnostic (suspicious_assignment) which
issues when an the compiler finds an assignment either where a
condition is expected or as the parameter to return.
master
Nicolas Hake 2018-03-27 08:12:21 +02:00
parent 99c14b5547
commit c73b8b3ece
4 changed files with 78 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
docs/sdk/script/Diagnostics.xml
View File

@ -229,6 +229,24 @@ func g(a, ...) {
</part>
</col>
</row>
<row>
<col>suspicious_assignment</col>
<col>
<text>
An assignment was found where an expression was expected. While
an assignment returns its own value, usually a comparison was
intended instead.
</text>
<part>
<code>if (a = b + 1) { /* Do something */}</code>
</part>
<text>
If the assignment is intentional, make this more obvious by
extracting it to a separate statement, or explicitly handle the
boolean conversion by adding a comparison operator.
</text>
</col>
</row>
</table>
</part>

14
src/script/C4AulCompiler.cpp
View File

@ -2,7 +2,7 @@
* OpenClonk, http://www.openclonk.org
*
* Copyright (c) 2001-2009, RedWolf Design GmbH, http://www.clonk.de/
* Copyright (c) 2009-2016, The OpenClonk Team and contributors
* Copyright (c) 2009-2018, The OpenClonk Team and contributors
*
* Distributed under the terms of the ISC license; see accompanying file
* "COPYING" for details.
@ -255,6 +255,14 @@ class C4AulCompiler::CodegenAstVisitor : public ::aul::DefaultRecursiveVisitor
}
};
void WarnOnAssignment(const ::aul::ast::ExprPtr &n) const
{
if (dynamic_cast<const ::aul::ast::AssignmentExpr*>(n.get()) != nullptr)
{
Warn(target_host, host, n.get(), Fn, C4AulWarningId::suspicious_assignment);
}
}
public:
CodegenAstVisitor(C4ScriptHost *host, C4ScriptHost *source_host) : target_host(host), host(source_host) {}
explicit CodegenAstVisitor(C4AulScriptFunc *func) : Fn(func), target_host(func->pOrgScript), host(target_host) {}
@ -1415,6 +1423,7 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::Return *n)
{
StackGuard g(this, 0);
WarnOnAssignment(n->value);
SafeVisit(n->value);
AddBCC(n->loc, AB_RETURN);
}
@ -1442,6 +1451,7 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::ForLoop *n)
PushLoop();
if (n->cond)
{
WarnOnAssignment(n->cond);
cond = AddJumpTarget();
SafeVisit(n->cond);
active_loops.top().breaks.push_back(AddBCC(n->cond->loc, AB_CONDN));
@ -1560,6 +1570,7 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::WhileLoop *n)
{
int cond = AddJumpTarget();
PushLoop();
WarnOnAssignment(n->cond);
SafeVisit(n->cond);
active_loops.top().breaks.push_back(AddBCC(n->cond->loc, AB_CONDN));
if (SafeVisit(n->body))
@ -1583,6 +1594,7 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::Continue *n)
void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::If *n)
{
WarnOnAssignment(n->cond);
SafeVisit(n->cond);
int jump = AddBCC(n->loc, AB_CONDN);
// Warn if we're controlling a no-op ("if (...);")

3
src/script/C4AulWarnings.h
View File

@ -1,7 +1,7 @@
/*
* OpenClonk, http://www.openclonk.org
*
* Copyright (c) 2017, The OpenClonk Team and contributors
* Copyright (c) 2017-2018, The OpenClonk Team and contributors
*
* Distributed under the terms of the ISC license; see accompanying file
* "COPYING" for details.
@ -38,5 +38,6 @@ DIAG(undeclared_varargs, "use of '%s' in a function forces it to take variable a
DIAG(arg_count_mismatch, "call to '%s' passes %u arguments, of which only %u are used", true)
DIAG(arg_type_mismatch, "parameter %u of call to '%s' passes %s (%s expected)", true)
DIAG(empty_if, "empty controlled statement (use '{}' if this is intentional)", true)
DIAG(suspicious_assignment, "suspicious assignment (was a comparison intended here?)", true)
#pragma pop_macro("DIAG")

46
tests/aul/AulDiagnosticsTest.cpp
View File

@ -1,7 +1,7 @@
/*
* OpenClonk, http://www.openclonk.org
*
* Copyright (c) 2015-2017, The OpenClonk Team and contributors
* Copyright (c) 2015-2018, The OpenClonk Team and contributors
*
* Distributed under the terms of the ISC license; see accompanying file
* "COPYING" for details.
@ -85,6 +85,50 @@ TEST_F(AulDiagnosticsTest, empty_if)
}
}
TEST_F(AulDiagnosticsTest, suspicious_assignment)
{
{
EXPECT_WARNING(suspicious_assignment);
RunCode("var a = 0; if (a = 0) {}");
}
{
EXPECT_WARNING(suspicious_assignment).Times(0);
RunCode("var a = 0; if (a == 1) {}");
RunCode("var a = 0; if (a += 1) {}");
}
{
EXPECT_WARNING(suspicious_assignment);
RunCode("for (var a = 0; a = 0;) {}");
}
{
EXPECT_WARNING(suspicious_assignment).Times(0);
RunCode("for (var a = 0; a == 1;) {}");
RunCode("for (var a = 0; a != 0;) {}");
}
{
EXPECT_WARNING(suspicious_assignment);
RunCode("var a = 0; while (a = 0) {}");
}
{
EXPECT_WARNING(suspicious_assignment).Times(0);
RunCode("var a = 0; while (a == 1) {}");
RunCode("var a = 0; while (a *= 1) {}");
}
{
EXPECT_WARNING(suspicious_assignment);
RunCode("var a = 0; return a = 0;");
}
{
EXPECT_WARNING(suspicious_assignment);
RunCode("var a = 0; return (a = 0);");
}
{
EXPECT_WARNING(suspicious_assignment).Times(0);
RunCode("var a = 0; return a == 0;");
RunCode("var a = 0; return (a = 0) == 0;");
}
}
TEST_F(AulDiagnosticsTest, arg_count_mismatch)
{
{