From 2ce67d5d5310166f9d06d88fb11d5d0896794404 Mon Sep 17 00:00:00 2001 From: James Fish Date: Thu, 23 Oct 2014 16:45:30 +0100 Subject: [PATCH 1/3] Fix line numbers detection in tests for OTP >= 17 --- test/lager_test_backend.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lager_test_backend.erl b/test/lager_test_backend.erl index dc772e8..71b9de9 100644 --- a/test/lager_test_backend.erl +++ b/test/lager_test_backend.erl @@ -109,7 +109,7 @@ print_bad_state() -> has_line_numbers() -> %% are we R15 or greater Rel = erlang:system_info(otp_release), - {match, [Major]} = re:run(Rel, "^R(\\d+)[A|B](|0(\\d))", [{capture, [1], list}]), + {match, [Major]} = re:run(Rel, "(?|(^R(\\d+)[A|B](|0(\\d)))|(^(\\d+)$))", [{capture, [2], list}]), list_to_integer(Major) >= 15. not_running_test() -> From 8685853c911bc9b29ec2165e120a30c00a5ee580 Mon Sep 17 00:00:00 2001 From: James Fish Date: Thu, 23 Oct 2014 21:42:23 +0100 Subject: [PATCH 2/3] Add maps formatting Note that compiling on OTP 17.0, or later, will produce beam files (specifically lager_trunc_io) that are incompatible with R16B03-1, or earlier, due to erlang:is_map/1 using a new beam opcode (156). However if compiled on R16B03-1, or earlier, will produce beams files that are compatible with 17.0, and later, and maps will be formatted where supported by the active OTP release. --- dialyzer.ignore-warnings | 5 ++ rebar.config | 2 +- src/lager_trunc_io.erl | 102 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/dialyzer.ignore-warnings b/dialyzer.ignore-warnings index e69de29..a0e8e5d 100644 --- a/dialyzer.ignore-warnings +++ b/dialyzer.ignore-warnings @@ -0,0 +1,5 @@ +lager_trunc_io.erl:283: Call to missing or unexported function erlang:is_map/1 +lager_trunc_io.erl:335: Call to missing or unexported function erlang:map_size/1 +Unknown functions: + lager_default_tracer:info/1 + maps:to_list/1 diff --git a/rebar.config b/rebar.config index 6460605..494a6ca 100644 --- a/rebar.config +++ b/rebar.config @@ -6,7 +6,7 @@ ]}. {xref_checks, []}. -{xref_queries, [{"(XC - UC) || (XU - X - B - lager_default_tracer : Mod)", []}]}. +{xref_queries, [{"(XC - UC) || (XU - X - B - lager_default_tracer : Mod - erlang:\"(is_map|map_size)\"/1 - maps:to_list/1)", []}]}. {cover_enabled, true}. {edoc_opts, [{stylesheet_file, "./priv/edoc.css"}]}. diff --git a/src/lager_trunc_io.erl b/src/lager_trunc_io.erl index a2f2d6b..7510143 100644 --- a/src/lager_trunc_io.erl +++ b/src/lager_trunc_io.erl @@ -277,6 +277,15 @@ print(List, Max, Options) when is_list(List) -> _ -> {R, Len} = list_body(List, Max - 2, dec_depth(Options), false), {[$[, R, $]], Len + 2} + end; + +print(Map, Max, Options) -> + case erlang:is_builtin(erlang, is_map, 1) andalso erlang:is_map(Map) of + true -> + {MapBody, Len} = map_body(Map, Max - 3, dec_depth(Options)), + {[$#, ${, MapBody, $}], Len + 3}; + false -> + error(badarg, [Map, Max, Options]) end. %% Returns {List, Length} @@ -322,6 +331,36 @@ list_bodyc(X, Max, Options, _Tuple) -> %% improper list {List, Len} = print(X, Max - 1, Options), {[$|,List], Len + 1}. +map_body(Map, Max, #print_options{depth=Depth}) when Max < 4; Depth =:= 0 -> + case erlang:map_size(Map) of + 0 -> {[], 0}; + _ -> {"...", 3} + end; +map_body(Map, Max, Options) -> + case maps:to_list(Map) of + [] -> + {[], 0}; + [{Key, Value} | Rest] -> + {KeyStr, KeyLen} = print(Key, Max - 4, Options), + DiffLen = KeyLen + 4, + {ValueStr, ValueLen} = print(Value, Max - DiffLen, Options), + DiffLen2 = DiffLen + ValueLen, + {Final, FLen} = map_bodyc(Rest, Max - DiffLen2, dec_depth(Options)), + {[KeyStr, " => ", ValueStr | Final], DiffLen2 + FLen} + end. + +map_bodyc([], _Max, _Options) -> + {[], 0}; +map_bodyc(_Rest, Max,#print_options{depth=Depth}) when Max < 5; Depth =:= 0 -> + {",...", 4}; +map_bodyc([{Key, Value} | Rest], Max, Options) -> + {KeyStr, KeyLen} = print(Key, Max - 5, Options), + DiffLen = KeyLen + 5, + {ValueStr, ValueLen} = print(Value, Max - DiffLen, Options), + DiffLen2 = DiffLen + ValueLen, + {Final, FLen} = map_bodyc(Rest, Max - DiffLen2, dec_depth(Options)), + {[$,, KeyStr, " => ", ValueStr | Final], DiffLen2 + FLen}. + %% The head of a list we hope is ascii. Examples: %% %% [65,66,67] -> "ABC" @@ -703,6 +742,46 @@ tuple_printing_test() -> 22835963083295358096932575511191922182123945984}], 53))), ok. +map_printing_test() -> + case erlang:is_builtin(erlang, is_map, 1) of + true -> + ?assertEqual("#{}", lists:flatten(format("~p", [maps:new()], 50))), + ?assertEqual("#{}", lists:flatten(format("~p", [maps:new()], 3))), + ?assertEqual("#{}", lists:flatten(format("~w", [maps:new()], 50))), + ?assertError(badarg, lists:flatten(format("~s", [maps:new()], 50))), + ?assertEqual("#{...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}])], 1))), + ?assertEqual("#{...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}])], 6))), + ?assertEqual("#{bar => ...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}])], 7))), + ?assertEqual("#{bar => ...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}])], 9))), + ?assertEqual("#{bar => foo}", lists:flatten(format("~p", [maps:from_list([{bar, foo}])], 10))), + ?assertEqual("#{bar => ...,...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}, {foo, bar}])], 9))), + ?assertEqual("#{bar => foo,...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}, {foo, bar}])], 10))), + ?assertEqual("#{bar => foo,...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}, {foo, bar}])], 17))), + ?assertEqual("#{bar => foo,foo => ...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}, {foo, bar}])], 18))), + ?assertEqual("#{bar => foo,foo => ...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}, {foo, bar}])], 19))), + ?assertEqual("#{bar => foo,foo => ...}", lists:flatten(format("~p", [maps:from_list([{bar, foo}, {foo, bar}])], 20))), + ?assertEqual("#{bar => foo,foo => bar}", lists:flatten(format("~p", [maps:from_list([{bar, foo}, {foo, bar}])], 21))), + ?assertEqual("#{22835963083295358096932575511191922182123945984 => ...}", + lists:flatten(format("~w", [ + maps:from_list([{22835963083295358096932575511191922182123945984, + 22835963083295358096932575511191922182123945984}])], 10))), + ?assertEqual("#{22835963083295358096932575511191922182123945984 => ...}", + lists:flatten(format("~w", [ + maps:from_list([{22835963083295358096932575511191922182123945984, + bar}])], 10))), + ?assertEqual("#{22835963083295358096932575511191922182123945984 => ...}", + lists:flatten(format("~w", [ + maps:from_list([{22835963083295358096932575511191922182123945984, + bar}])], 53))), + ?assertEqual("#{22835963083295358096932575511191922182123945984 => bar}", + lists:flatten(format("~w", [ + maps:from_list([{22835963083295358096932575511191922182123945984, + bar}])], 54))), + ok; + false -> + ok + end. + unicode_test() -> ?assertEqual([231,167,129], lists:flatten(format("~s", [<<231,167,129>>], 50))), ?assertEqual([31169], lists:flatten(format("~ts", [<<231,167,129>>], 50))), @@ -726,6 +805,29 @@ depth_limit_test() -> ?assertEqual("{a,{b,{c,{...}}}}", lists:flatten(format("~P", [{a, {b, {c, {d}}}}, 7], 50))), ?assertEqual("{a,{b,{c,{d}}}}", lists:flatten(format("~P", [{a, {b, {c, {d}}}}, 8], 50))), + case erlang:is_builtin(erlang, is_map, 1) of + true -> + ?assertEqual("#{a => #{...}}", + lists:flatten(format("~P", + [maps:from_list([{a, maps:from_list([{b, maps:from_list([{c, d}])}])}]), 2], 50))), + ?assertEqual("#{a => #{b => #{...}}}", + lists:flatten(format("~P", + [maps:from_list([{a, maps:from_list([{b, maps:from_list([{c, d}])}])}]), 3], 50))), + ?assertEqual("#{a => #{b => #{c => d}}}", + lists:flatten(format("~P", + [maps:from_list([{a, maps:from_list([{b, maps:from_list([{c, d}])}])}]), 4], 50))), + + ?assertEqual("#{}", lists:flatten(format("~P", [maps:new(), 1], 50))), + ?assertEqual("#{...}", lists:flatten(format("~P", [maps:from_list([{1,1}, {2,2}, {3,3}]), 1], 50))), + ?assertEqual("#{1 => 1,...}", lists:flatten(format("~P", [maps:from_list([{1,1}, {2,2}, {3,3}]), 2], 50))), + ?assertEqual("#{1 => 1,2 => 2,...}", lists:flatten(format("~P", [maps:from_list([{1,1}, {2,2}, {3,3}]), 3], 50))), + ?assertEqual("#{1 => 1,2 => 2,3 => 3}", lists:flatten(format("~P", [maps:from_list([{1,1}, {2,2}, {3,3}]), 4], 50))), + + ok; + false -> + ok + end, + ?assertEqual("{\"a\",[...]}", lists:flatten(format("~P", [{"a", ["b", ["c", ["d"]]]}, 3], 50))), ?assertEqual("{\"a\",[\"b\",[[...]|...]]}", lists:flatten(format("~P", [{"a", ["b", ["c", ["d"]]]}, 6], 50))), ?assertEqual("{\"a\",[\"b\",[\"c\",[\"d\"]]]}", lists:flatten(format("~P", [{"a", ["b", ["c", ["d"]]]}, 9], 50))), From cd76e0fff041fc32104ac5d547df41c15cc7280c Mon Sep 17 00:00:00 2001 From: James Fish Date: Sat, 25 Oct 2014 09:31:48 +0100 Subject: [PATCH 3/3] Update tools.mk --- .gitignore | 2 + tools.mk | 151 ++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 128 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index d053947..7f9053c 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,5 @@ erl_crash.dump log deps .local_dialyzer_plt +dialyzer_unhandled_warnings +dialyzer_warnings diff --git a/tools.mk b/tools.mk index 8e0e1b9..ab05649 100644 --- a/tools.mk +++ b/tools.mk @@ -1,48 +1,149 @@ +# ------------------------------------------------------------------- +# +# Copyright (c) 2014 Basho Technologies, Inc. +# +# This file is provided to you under the Apache License, +# Version 2.0 (the "License"); you may not use this file +# except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# ------------------------------------------------------------------- + +# ------------------------------------------------------------------- +# NOTE: This file is is from https://github.com/basho/tools.mk. +# It should not be edited in a project. It should simply be updated +# wholesale when a new version of tools.mk is released. +# ------------------------------------------------------------------- + +REBAR ?= ./rebar +REVISION ?= $(shell git rev-parse --short HEAD) +PROJECT ?= $(shell basename `find src -name "*.app.src"` .app.src) + +.PHONY: compile-no-deps test docs xref dialyzer-run dialyzer-quick dialyzer \ + cleanplt upload-docs + +compile-no-deps: + ${REBAR} compile skip_deps=true + test: compile - ./rebar eunit skip_deps=true + ${REBAR} eunit skip_deps=true + +upload-docs: docs + @if [ -z "${BUCKET}" -o -z "${PROJECT}" -o -z "${REVISION}" ]; then \ + echo "Set BUCKET, PROJECT, and REVISION env vars to upload docs"; \ + exit 1; fi + @cd doc; s3cmd put -P * "s3://${BUCKET}/${PROJECT}/${REVISION}/" > /dev/null + @echo "Docs built at: http://${BUCKET}.s3-website-us-east-1.amazonaws.com/${PROJECT}/${REVISION}" docs: - ./rebar doc skip_deps=true + ${REBAR} doc skip_deps=true xref: compile - ./rebar xref skip_deps=true + ${REBAR} xref skip_deps=true -PLT ?= $(HOME)/.riak_combo_dialyzer_plt +PLT ?= $(HOME)/.combo_dialyzer_plt LOCAL_PLT = .local_dialyzer_plt DIALYZER_FLAGS ?= -Wunmatched_returns ${PLT}: compile -ifneq (,$(wildcard $(PLT))) - dialyzer --check_plt --plt $(PLT) --apps $(DIALYZER_APPS) && \ - dialyzer --add_to_plt --plt $(PLT) --output_plt $(PLT) --apps $(DIALYZER_APPS) ; test $$? -ne 1 -else - dialyzer --build_plt --output_plt $(PLT) --apps $(DIALYZER_APPS); test $$? -ne 1 -endif + @if [ -f $(PLT) ]; then \ + dialyzer --check_plt --plt $(PLT) --apps $(DIALYZER_APPS) && \ + dialyzer --add_to_plt --plt $(PLT) --output_plt $(PLT) --apps $(DIALYZER_APPS) ; test $$? -ne 1; \ + else \ + dialyzer --build_plt --output_plt $(PLT) --apps $(DIALYZER_APPS); test $$? -ne 1; \ + fi ${LOCAL_PLT}: compile -ifneq (,$(wildcard deps/*)) -ifneq (,$(wildcard $(LOCAL_PLT))) - dialyzer --check_plt --plt $(LOCAL_PLT) deps/*/ebin && \ - dialyzer --add_to_plt --plt $(LOCAL_PLT) --output_plt $(LOCAL_PLT) deps/*/ebin ; test $$? -ne 1 -else - dialyzer --build_plt --output_plt $(LOCAL_PLT) deps/*/ebin ; test $$? -ne 1 -endif -endif - -dialyzer: ${PLT} ${LOCAL_PLT} + @if [ -d deps ]; then \ + if [ -f $(LOCAL_PLT) ]; then \ + dialyzer --check_plt --plt $(LOCAL_PLT) deps/*/ebin && \ + dialyzer --add_to_plt --plt $(LOCAL_PLT) --output_plt $(LOCAL_PLT) deps/*/ebin ; test $$? -ne 1; \ + else \ + dialyzer --build_plt --output_plt $(LOCAL_PLT) deps/*/ebin ; test $$? -ne 1; \ + fi \ + fi + +dialyzer-run: @echo "==> $(shell basename $(shell pwd)) (dialyzer)" +# The bulk of the code below deals with the dialyzer.ignore-warnings file +# which contains strings to ignore if output by dialyzer. +# Typically the strings include line numbers. Using them exactly is hard +# to maintain as the code changes. This approach instead ignores the line +# numbers, but takes into account the number of times a string is listed +# for a given file. So if one string is listed once, for example, and it +# appears twice in the warnings, the user is alerted. It is possible but +# unlikely that this approach could mask a warning if one ignored warning +# is removed and two warnings of the same kind appear in the file, for +# example. But it is a trade-off that seems worth it. +# Details of the cryptic commands: +# - Remove line numbers from dialyzer.ignore-warnings +# - Pre-pend duplicate count to each warning with sort | uniq -c +# - Remove annoying white space around duplicate count +# - Save in dialyer.ignore-warnings.tmp +# - Do the same to dialyzer_warnings +# - Remove matches from dialyzer.ignore-warnings.tmp from output +# - Remove duplicate count +# - Escape regex special chars to use lines as regex patterns +# - Add pattern to match any line number (file.erl:\d+:) +# - Anchor to match the entire line (^entire line$) +# - Save in dialyzer_unhandled_warnings +# - Output matches for those patterns found in the original warnings @if [ -f $(LOCAL_PLT) ]; then \ - dialyzer $(DIALYZER_FLAGS) --plts $(PLT) $(LOCAL_PLT) -c ebin; \ + PLTS="$(PLT) $(LOCAL_PLT)"; \ + else \ + PLTS=$(PLT); \ + fi; \ + if [ -f dialyzer.ignore-warnings ]; then \ + if [ $$(grep -cvE '[^[:space:]]' dialyzer.ignore-warnings) -ne 0 ]; then \ + echo "ERROR: dialyzer.ignore-warnings contains a blank/empty line, this will match all messages!"; \ + exit 1; \ + fi; \ + dialyzer $(DIALYZER_FLAGS) --plts $${PLTS} -c ebin > dialyzer_warnings ; \ + cat dialyzer.ignore-warnings \ + | sed -E 's/^([^:]+:)[^:]+:/\1/' \ + | sort \ + | uniq -c \ + | sed -E '/.*\.erl: /!s/^[[:space:]]*[0-9]+[[:space:]]*//' \ + > dialyzer.ignore-warnings.tmp ; \ + egrep -v "^[[:space:]]*(done|Checking|Proceeding|Compiling)" dialyzer_warnings \ + | sed -E 's/^([^:]+:)[^:]+:/\1/' \ + | sort \ + | uniq -c \ + | sed -E '/.*\.erl: /!s/^[[:space:]]*[0-9]+[[:space:]]*//' \ + | grep -F -f dialyzer.ignore-warnings.tmp -v \ + | sed -E 's/^[[:space:]]*[0-9]+[[:space:]]*//' \ + | sed -E 's/([]\^:+?|()*.$${}\[])/\\\1/g' \ + | sed -E 's/(\\\.erl\\\:)/\1\\d+:/g' \ + | sed -E 's/^(.*)$$/^\1$$/g' \ + > dialyzer_unhandled_warnings ; \ + rm dialyzer.ignore-warnings.tmp; \ + if [ $$(cat dialyzer_unhandled_warnings | wc -l) -gt 0 ]; then \ + egrep -f dialyzer_unhandled_warnings dialyzer_warnings ; \ + found_warnings=1; \ + fi; \ + [ "$$found_warnings" != 1 ] ; \ else \ - dialyzer $(DIALYZER_FLAGS) --plts $(PLT) -c ebin; \ + dialyzer $(DIALYZER_FLAGS) --plts $${PLTS} -c ebin; \ fi +dialyzer-quick: compile-no-deps dialyzer-run + +dialyzer: ${PLT} ${LOCAL_PLT} dialyzer-run + cleanplt: - @echo + @echo @echo "Are you sure? It takes several minutes to re-build." @echo Deleting $(PLT) and $(LOCAL_PLT) in 5 seconds. - @echo + @echo sleep 5 rm $(PLT) rm $(LOCAL_PLT) -