Skip to content

Commit 41ba954

Browse files
AndreaWalchshoferSCCHAndreaWalchshoferSCCH
committed
Incorporate review comments
1 parent 0fed6a8 commit 41ba954

File tree

9 files changed

+159
-141
lines changed

9 files changed

+159
-141
lines changed

‎CodeFormatCore/include/CodeFormatCore/Format/Analyzer/FormatStrategy.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,4 @@ enum class SemicolonStrategy {
128128
Add,
129129
Remove,
130130
InsertNewLine
131-
};
131+
};

‎CodeFormatCore/include/CodeFormatCore/Format/Analyzer/SemicolonAnalyzer.h‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,22 @@
77

88
class SemicolonAnalyzer : public FormatAnalyzer {
99
public:
10-
DECLARE_FORMAT_ANALYZER(SemicolonAnalyzer)
10+
DECLARE_FORMAT_ANALYZER(SemicolonAnalyzer)
1111

12-
SemicolonAnalyzer();
12+
SemicolonAnalyzer();
1313

14-
void Analyze(FormatState& f, const LuaSyntaxTree& t) override;
14+
void Analyze(FormatState &f, const LuaSyntaxTree &t) override;
1515

16-
void Query(FormatState& f, LuaSyntaxNode syntaxNode, const LuaSyntaxTree& t, FormatResolve& resolve) override;
16+
void Query(FormatState &f, LuaSyntaxNode syntaxNode, const LuaSyntaxTree &t, FormatResolve &resolve) override;
1717

1818
private:
19-
void AddSemicolon(LuaSyntaxNode n, const LuaSyntaxTree& t);
20-
void InsertNewLineBeforeNode(LuaSyntaxNode n, const LuaSyntaxTree& t);
21-
void RemoveSemicolon(LuaSyntaxNode n, const LuaSyntaxTree& t);
22-
bool IsFirstStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree& t);
23-
bool IsLastStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree& t);
24-
bool EndsWithSemicolon(LuaSyntaxNode n, const LuaSyntaxTree& t);
25-
LuaSyntaxNode GetLastNonCommentToken(LuaSyntaxNode n, const LuaSyntaxTree& t);
19+
void AddSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t);
20+
void InsertNewLineBeforeNode(LuaSyntaxNode n, const LuaSyntaxTree &t);
21+
void RemoveSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t);
22+
bool IsFirstStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t);
23+
bool IsLastStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t);
24+
bool EndsWithSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t);
25+
LuaSyntaxNode GetLastNonCommentToken(LuaSyntaxNode n, const LuaSyntaxTree &t);
2626

27-
std::unordered_map<std::size_t, SemicolonStrategy> _semicolon;
27+
std::unordered_map<std::size_t, SemicolonStrategy> _semicolon;
2828
};

‎CodeFormatCore/src/Config/LuaStyle.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ void LuaStyle::ParseFromMap(std::map<std::string, std::string, std::less<>> &con
276276
end_statement_with_semicolon = EndStmtWithSemicolon::Always;
277277
} else if (configMap.at("end_statement_with_semicolon") == "never") {
278278
end_statement_with_semicolon = EndStmtWithSemicolon::Never;
279-
} else if (configMap.at("end_statement_with_semicolon") == "sameLine") {
279+
} else if (configMap.at("end_statement_with_semicolon") == "same_line") {
280280
end_statement_with_semicolon = EndStmtWithSemicolon::SameLine;
281281
}
282282
}

‎CodeFormatCore/src/Diagnostic/CodeStyle/CodeStyleChecker.cpp‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,19 @@ void CodeStyleChecker::BasicResolve(LuaSyntaxNode syntaxNode, const LuaSyntaxTre
9696
}
9797
case TokenStrategy::StmtEndSemicolon: {
9898
switch (d.GetState().GetStyle().end_statement_with_semicolon) {
99-
case EndStmtWithSemicolon::Never:
99+
case EndStmtWithSemicolon::Never: {
100100
d.PushDiagnostic(DiagnosticType::Semicolon, textRange,
101-
LText("expected statement not to end with ;"));
101+
LText("expected statement not to end with ;"));
102102
break;
103-
case EndStmtWithSemicolon::SameLine:
103+
}
104+
case EndStmtWithSemicolon::SameLine: {
104105
d.PushDiagnostic(DiagnosticType::Semicolon, textRange,
105-
LText("; should only separate multiple statements on a single line"));
106+
LText("; should only separate multiple statements on a single line"));
106107
break;
107-
default:
108+
}
109+
default: {
108110
break;
111+
}
109112
}
110113
break;
111114
}
@@ -115,13 +118,15 @@ void CodeStyleChecker::BasicResolve(LuaSyntaxNode syntaxNode, const LuaSyntaxTre
115118
}
116119

117120
switch (resolve.GetTokenAddStrategy()) {
118-
case TokenAddStrategy::StmtEndSemicolon:
119-
d.PushDiagnostic(DiagnosticType::Semicolon,
120-
TextRange(textRange.GetEndOffset(), 1),
121-
LText("expected ; at end of statement"));
121+
case TokenAddStrategy::StmtEndSemicolon: {
122+
d.PushDiagnostic(DiagnosticType::Semicolon,
123+
TextRange(textRange.GetEndOffset(), 1),
124+
LText("expected ; at end of statement"));
122125
break;
123-
default:
126+
}
127+
default: {
124128
break;
129+
}
125130
}
126131

127132
switch (resolve.GetNextSpaceStrategy()) {

‎CodeFormatCore/src/Format/Analyzer/SemicolonAnalyzer.cpp‎

Lines changed: 111 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,130 +1,135 @@
11
#include "CodeFormatCore/Format/Analyzer/SemicolonAnalyzer.h"
2-
#include "CodeFormatCore/Format/FormatState.h"
32
#include "CodeFormatCore/Config/LuaStyleEnum.h"
4-
#include <LuaParser/Lexer/LuaTokenTypeDetail.h>
3+
#include "CodeFormatCore/Format/FormatState.h"
4+
#include "LuaParser/Lexer/LuaTokenTypeDetail.h"
55

66
SemicolonAnalyzer::SemicolonAnalyzer() {
77
}
88

9-
void SemicolonAnalyzer::Analyze(FormatState& f, const LuaSyntaxTree& t) {
10-
if (f.GetStyle().end_statement_with_semicolon == EndStmtWithSemicolon::Keep) {
11-
return; // No analysis needed
12-
}
9+
void SemicolonAnalyzer::Analyze(FormatState &f, const LuaSyntaxTree &t) {
10+
if (f.GetStyle().end_statement_with_semicolon == EndStmtWithSemicolon::Keep) {
11+
return;// No analysis needed
12+
}
1313

14-
for (auto syntaxNode : t.GetSyntaxNodes()) {
15-
if (syntaxNode.IsNode(t)) {
16-
if (detail::multi_match::StatementMatch(syntaxNode.GetSyntaxKind(t))) {
17-
auto text = syntaxNode.GetText(t);
18-
switch (f.GetStyle().end_statement_with_semicolon) {
19-
case EndStmtWithSemicolon::Always:
20-
if (!EndsWithSemicolon(syntaxNode, t)) {
21-
AddSemicolon(syntaxNode, t);
22-
}
23-
break;
24-
case EndStmtWithSemicolon::Never:
25-
// is on same line as other statement -> needs to go on new line
26-
if (!IsFirstStmtOfLine(syntaxNode, t)) {
27-
InsertNewLineBeforeNode(syntaxNode, t);
28-
}
29-
if (EndsWithSemicolon(syntaxNode, t)) {
30-
RemoveSemicolon(syntaxNode, t);
31-
}
32-
break;
33-
case EndStmtWithSemicolon::SameLine:
34-
if (EndsWithSemicolon(syntaxNode, t)) {
35-
if (IsLastStmtOfLine(syntaxNode, t)) {
36-
RemoveSemicolon(syntaxNode, t);
37-
}
38-
}
39-
break;
40-
default:
41-
break;
42-
}
43-
}
44-
}
45-
}
14+
for (auto syntaxNode: t.GetSyntaxNodes()) {
15+
if (syntaxNode.IsNode(t)) {
16+
if (detail::multi_match::StatementMatch(syntaxNode.GetSyntaxKind(t))) {
17+
switch (f.GetStyle().end_statement_with_semicolon) {
18+
case EndStmtWithSemicolon::Always: {
19+
if (!EndsWithSemicolon(syntaxNode, t)) {
20+
AddSemicolon(syntaxNode, t);
21+
}
22+
break;
23+
}
24+
case EndStmtWithSemicolon::Never: {
25+
// is on same line as other statement -> needs to go on new line
26+
if (!IsFirstStmtOfLine(syntaxNode, t)) {
27+
InsertNewLineBeforeNode(syntaxNode, t);
28+
}
29+
if (EndsWithSemicolon(syntaxNode, t)) {
30+
RemoveSemicolon(syntaxNode, t);
31+
}
32+
break;
33+
}
34+
case EndStmtWithSemicolon::SameLine: {
35+
if (EndsWithSemicolon(syntaxNode, t)) {
36+
if (IsLastStmtOfLine(syntaxNode, t)) {
37+
RemoveSemicolon(syntaxNode, t);
38+
}
39+
}
40+
break;
41+
}
42+
default: {
43+
break;
44+
}
45+
}
46+
}
47+
}
48+
}
4649
}
4750

48-
void SemicolonAnalyzer::Query(FormatState& f, LuaSyntaxNode n, const LuaSyntaxTree& t, FormatResolve& resolve) {
49-
auto it = _semicolon.find(n.GetIndex());
50-
if (it != _semicolon.end()) {
51-
auto& strategy = it->second;
52-
switch (strategy) {
53-
case SemicolonStrategy::Add:
54-
resolve.SetTokenAddStrategy(TokenAddStrategy::StmtEndSemicolon);
55-
break;
56-
case SemicolonStrategy::Remove:
57-
resolve.SetTokenStrategy(TokenStrategy::StmtEndSemicolon);
58-
break;
59-
case SemicolonStrategy::InsertNewLine:
60-
resolve.SetTokenStrategy(TokenStrategy::NewLineBeforeToken);
61-
break;
62-
default:
63-
break;
64-
}
65-
}
51+
void SemicolonAnalyzer::Query(FormatState &f, LuaSyntaxNode n, const LuaSyntaxTree &t, FormatResolve &resolve) {
52+
auto it = _semicolon.find(n.GetIndex());
53+
if (it != _semicolon.end()) {
54+
auto &strategy = it->second;
55+
switch (strategy) {
56+
case SemicolonStrategy::Add: {
57+
resolve.SetTokenAddStrategy(TokenAddStrategy::StmtEndSemicolon);
58+
break;
59+
}
60+
case SemicolonStrategy::Remove: {
61+
resolve.SetTokenStrategy(TokenStrategy::StmtEndSemicolon);
62+
break;
63+
}
64+
case SemicolonStrategy::InsertNewLine: {
65+
resolve.SetTokenStrategy(TokenStrategy::NewLineBeforeToken);
66+
break;
67+
}
68+
default: {
69+
break;
70+
}
71+
}
72+
}
6673
}
6774

68-
void SemicolonAnalyzer::AddSemicolon(LuaSyntaxNode n, const LuaSyntaxTree& t) {
69-
auto token = GetLastNonCommentToken(n, t);
70-
if (token.IsToken(t)) {
71-
_semicolon[token.GetIndex()] = SemicolonStrategy::Add;
72-
}
75+
void SemicolonAnalyzer::AddSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t) {
76+
auto token = GetLastNonCommentToken(n, t);
77+
if (token.IsToken(t)) {
78+
_semicolon[token.GetIndex()] = SemicolonStrategy::Add;
79+
}
7380
}
7481

75-
void SemicolonAnalyzer::InsertNewLineBeforeNode(LuaSyntaxNode n, const LuaSyntaxTree& t) {
76-
auto token = n.GetFirstToken(t); // line breaks are put in front of the statement itself by non-first statements
77-
if (token.IsToken(t)) {
78-
_semicolon[token.GetIndex()] = SemicolonStrategy::InsertNewLine;
79-
}
82+
void SemicolonAnalyzer::InsertNewLineBeforeNode(LuaSyntaxNode n, const LuaSyntaxTree &t) {
83+
auto token = n.GetFirstToken(t); // line breaks are put in front of the statement itself by non-first statements
84+
if (token.IsToken(t)) {
85+
_semicolon[token.GetIndex()] = SemicolonStrategy::InsertNewLine;
86+
}
8087
}
8188

82-
void SemicolonAnalyzer::RemoveSemicolon(LuaSyntaxNode n, const LuaSyntaxTree& t) {
83-
auto token = GetLastNonCommentToken(n, t);
84-
if (token.IsToken(t)) {
85-
_semicolon[token.GetIndex()] = SemicolonStrategy::Remove;
86-
}
89+
void SemicolonAnalyzer::RemoveSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t) {
90+
auto token = GetLastNonCommentToken(n, t);
91+
if (token.IsToken(t)) {
92+
_semicolon[token.GetIndex()] = SemicolonStrategy::Remove;
93+
}
8794
}
8895

89-
bool SemicolonAnalyzer::IsFirstStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree& t) {
90-
// check if current stmt starts on same line as the previous one ends
91-
auto startCurrent = n.GetStartLine(t);
92-
auto prev = n.GetPrevToken(t);
93-
if (!prev.IsNull(t)) {
94-
auto endPrev = prev.GetEndLine(t);
95-
return endPrev < startCurrent;
96-
}
97-
return true; // there's no previous token
96+
bool SemicolonAnalyzer::IsFirstStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t) {
97+
// check if current stmt starts on same line as the previous one ends
98+
auto startCurrent = n.GetStartLine(t);
99+
auto prev = n.GetPrevToken(t);
100+
if (!prev.IsNull(t)) {
101+
auto endPrev = prev.GetEndLine(t);
102+
return endPrev < startCurrent;
103+
}
104+
return true;// there's no previous token
98105
}
99106

100-
bool SemicolonAnalyzer::IsLastStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree& t) {
101-
// check if next stmt starts on same line as the current one ends
102-
auto currentEnd = n.GetEndLine(t);
103-
auto nextToken = n.GetNextToken(t);
104-
if (!nextToken.IsNull(t)) {
105-
auto nextStart = nextToken.GetStartLine(t);
106-
return currentEnd != nextStart;
107-
}
108-
return true; // there's no next token
107+
bool SemicolonAnalyzer::IsLastStmtOfLine(LuaSyntaxNode n, const LuaSyntaxTree &t) {
108+
// check if next stmt starts on same line as the current one ends
109+
auto currentEnd = n.GetEndLine(t);
110+
auto nextToken = n.GetNextToken(t);
111+
if (!nextToken.IsNull(t)) {
112+
auto nextStart = nextToken.GetStartLine(t);
113+
return currentEnd != nextStart;
114+
}
115+
return true;// there's no next token
109116
}
110117

111-
bool SemicolonAnalyzer::EndsWithSemicolon(LuaSyntaxNode n, const LuaSyntaxTree& t) {
112-
auto token = GetLastNonCommentToken(n, t);
113-
auto text = token.GetText(t);
114-
115-
return text == ";";
118+
bool SemicolonAnalyzer::EndsWithSemicolon(LuaSyntaxNode n, const LuaSyntaxTree &t) {
119+
auto token = GetLastNonCommentToken(n, t);
120+
return token.GetTokenKind(t) == ';';
116121
}
117122

118-
LuaSyntaxNode SemicolonAnalyzer::GetLastNonCommentToken(LuaSyntaxNode n, const LuaSyntaxTree& t) {
119-
auto token = n.GetLastToken(t);
120-
switch (token.GetTokenKind(t)) {
121-
case TK_SHORT_COMMENT:
122-
case TK_LONG_COMMENT:
123-
case TK_SHEBANG: {
124-
return token.GetPrevToken(t);
125-
}
126-
default: {
127-
return token;
128-
}
129-
}
123+
LuaSyntaxNode SemicolonAnalyzer::GetLastNonCommentToken(LuaSyntaxNode n, const LuaSyntaxTree &t) {
124+
auto token = n.GetLastToken(t);
125+
switch (token.GetTokenKind(t)) {
126+
case TK_SHORT_COMMENT:
127+
case TK_LONG_COMMENT:
128+
case TK_SHEBANG: {
129+
return token.GetPrevToken(t);
130+
}
131+
default: {
132+
return token;
133+
}
134+
}
130135
}

‎CodeFormatCore/src/Format/FormatState.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#include "CodeFormatCore/Format/Analyzer/LineBreakAnalyzer.h"
66
#include "CodeFormatCore/Format/Analyzer/SpaceAnalyzer.h"
77
#include "CodeFormatCore/Format/Analyzer/TokenAnalyzer.h"
8-
#include <CodeFormatCore/Format/Analyzer/SemicolonAnalyzer.h>
8+
#include "CodeFormatCore/Format/Analyzer/SemicolonAnalyzer.h"
99

1010
FormatState::FormatState(Mode mode)
1111
: _currentWidth(0),

0 commit comments

Comments
 (0)