-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Closes #236] Operator space false positive #259
Changes from 7 commits
c0b780d
4cf1ca6
1f17bd0
02c63c6
c798bb5
240fe72
8963e71
b331d65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -609,12 +609,17 @@ check_operator_spaces_rule(Line, Num, {Position, Operator}, Root) -> | |
not_found -> undefined; | ||
{ok, Node} -> ktn_code:type(Node) | ||
end, | ||
case Type of | ||
atom -> []; | ||
binary_element -> []; | ||
string -> []; | ||
char -> []; | ||
comment -> []; | ||
TokenType = case elvis_code:find_token(Root, {Num, Col}) of | ||
not_found -> undefined; | ||
{ok, Token} -> ktn_code:type(Token) | ||
end, | ||
case {Type, TokenType} of | ||
{atom, _} -> []; | ||
{binary_element, _} -> []; | ||
{string, _} -> []; | ||
{char, _} -> []; | ||
{comment, _} -> []; | ||
{_, string} -> []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why matching all this cases? If I'm not mistaken it would either be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have to check for those types of nodes or tokens where an operator without a space is acceptable. An alternative would be to match for all that shouldn't have it, but in either case there needs to be an explicit specification of the valid (or invalid) types of nodes/tokens. |
||
_ -> | ||
Msg = ?OPERATOR_SPACE_MSG, | ||
Info = [Label, Operator, Num], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
-module(fail_operator_spaces). | ||
|
||
-export([function1/2,function2/2, function3/2, function4/2, function5/0]). | ||
-export([function1/2,function2/2, function3/2, function4/2, function5/0, tag_filters/2]). | ||
|
||
%% No space before and after coma,on a comment. | ||
|
||
|
@@ -23,4 +23,24 @@ function4(Should, <<_:10/binary, ",", _/binary>>) -> | |
|
||
function5() -> | ||
User = #{name => <<"Juan">>, email => <<"juan@inaka.com">>}, | ||
<<"juan@inaka.com">> = maps:get(email,User). | ||
<<"juan@inaka.com">> = maps:get(email,User). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you using 2 or 4 spaces? Line 25 is using 4. |
||
|
||
tag_filters(DocName, #{conn := Conn} = State) -> | ||
TableName = atom_to_list(DocName), | ||
Sql = ["SELECT " | ||
" 'tag' AS \"type\", " | ||
" tag_name AS value, " | ||
" COUNT(1) AS \"count\" " | ||
"FROM ( " | ||
" SELECT unnest(regexp_split_to_array(tags, ',')) AS tag_name" | ||
" FROM ", TableName, " " | ||
") AS tags " | ||
"GROUP BY tag_name " | ||
"ORDER BY tag_name "], | ||
Values = [], | ||
case {Conn, Sql, Values} of | ||
{ok, Maps} -> | ||
{ok, {raw, Maps}, State}; | ||
{error, Error} -> | ||
{error, Error, State} | ||
end. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
-module(xref_SUITE). | ||
|
||
-type config() :: proplists:proplist(). | ||
|
||
-export( | ||
[ | ||
all/0, | ||
xref/1 | ||
] | ||
). | ||
|
||
-spec all() -> [atom()]. | ||
all() -> | ||
ExcludedFuns = [all, module_info], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why exclude There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a comprehensive exclusion list. |
||
Exports = ?MODULE:module_info(exports), | ||
[F || {F, 1} <- Exports, not lists:member(F, ExcludedFuns)]. | ||
|
||
-spec xref(config()) -> {comment, []}. | ||
xref(_Config) -> | ||
Dirs = [filename:absname("../../ebin")], | ||
[] = xref_runner:check(undefined_function_calls, #{dirs => Dirs}), | ||
[] = xref_runner:check(undefined_functions, #{dirs => Dirs}), | ||
[] = xref_runner:check(locals_not_used, #{dirs => Dirs}), | ||
[] = xref_runner:check(deprecated_function_calls, #{dirs => Dirs}), | ||
[] = xref_runner:check(deprecated_functions, #{dirs => Dirs}), | ||
{comment, ""}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This spec says it returns undefined, but it returns
not_found
and it's converted to undefined elvis_style.erl@609