Pretty print statements #1280

turbolent · 2021-11-29T01:25:45Z

Work towards #209

Description

Implement pretty-printing functions for all statement AST nodes:

ReturnStatement
BreakStatement
ContinueStatement
IfStatement
WhileStatement
ForStatement
EmitStatement
AssignmentStatement
SwapStatement
ExpressionStatement

Also:

Add missing JSON marshaling tests
Allow inserting tabs (4 spaces) in the pretty printer tool's text area

Targeted PR against master branch
Linked to Github issue with discussion and accepted design OR link to spec that describes this work
Code follows the standards mentioned here
Updated relevant documentation
Re-reviewed Files changed in the Github PR explorer
Added appropriate labels

codecov-commenter · 2021-11-30T00:50:00Z

Codecov Report

Merging #1280 (f60a37f) into master (7091617) will increase coverage by 0.12%.
The diff coverage is 98.56%.

@@            Coverage Diff             @@
##           master    #1280      +/-   ##
==========================================
+ Coverage   77.22%   77.35%   +0.12%     
==========================================
  Files         278      278              
  Lines       35814    35932     +118     
==========================================
+ Hits        27659    27796     +137     
+ Misses       7069     7050      -19     
  Partials     1086     1086

Flag	Coverage Δ
unittests	`77.35% <98.56%> (+0.12%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
tools/pretty/main.go	`0.00% <ø> (ø)`
runtime/ast/block.go	`89.79% <89.47%> (-0.21%)`	⬇️
runtime/ast/expression.go	`82.89% <100.00%> (+0.01%)`	⬆️
runtime/ast/statement.go	`92.98% <100.00%> (+14.72%)`	⬆️
runtime/ast/transfer.go	`80.00% <0.00%> (+15.00%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7091617...f60a37f. Read the comment docs.

github-actions · 2021-11-30T00:50:06Z

Cadence Benchstat comparison

This branch with compared with the base branch onflow:master commit 7091617
The command go test ./... -run=XXX -bench=. -shuffle=on -count N was used.
Bench tests were run a total of 7 times on each branch.

Results

	old.txt	new.txt
	time/op		delta
RuntimeFungibleTokenTransfer-2	1.14ms ±25%	1.07ms ± 2%	~	(p=0.628 n=7+6)
RuntimeResourceDictionaryValues-2	14.4ms ± 2%	15.7ms ±11%	~	(p=0.101 n=6+7)
ParseDeploy/byte_array-2	32.7ms ± 6%	32.3ms ± 5%	~	(p=0.534 n=6+7)
ParseDeploy/decode_hex-2	1.30ms ± 2%	1.29ms ± 4%	~	(p=0.620 n=7+7)
ParseArray-2	20.4ms ± 2%	20.9ms ± 5%	~	(p=0.073 n=7+7)
QualifiedIdentifierCreation/One_level-2	2.91ns ± 5%	2.84ns ± 6%	~	(p=0.155 n=7+7)
QualifiedIdentifierCreation/Three_levels-2	146ns ± 4%	143ns ± 4%	~	(p=0.318 n=7+7)
CheckContractInterfaceFungibleTokenConformance-2	161µs ±10%	154µs ± 3%	~	(p=0.073 n=7+7)
ContractInterfaceFungibleToken-2	43.4µs ± 4%	42.9µs ± 3%	~	(p=0.534 n=7+6)
NewInterpreter/new_interpreter-2	1.04µs ± 2%	1.04µs ± 2%	~	(p=0.810 n=6+7)
InterpretRecursionFib-2	2.41ms ± 2%	2.41ms ± 5%	~	(p=0.945 n=6+7)
ParseFungibleToken-2	434µs ± 4%	418µs ± 4%	−3.81%	(p=0.022 n=6+7)
ParseInfix-2	22.6µs ± 3%	21.5µs ± 4%	−4.89%	(p=0.002 n=7+7)
NewInterpreter/new_sub-interpreter-2	1.98µs ± 2%	1.88µs ± 5%	−5.18%	(p=0.002 n=7+7)
RuntimeStorageWriteCached-2	139µs ± 4%	123µs ± 3%	−11.51%	(p=0.001 n=6+7)

	alloc/op		delta
RuntimeFungibleTokenTransfer-2	233kB ± 0%	233kB ± 0%	~	(p=1.000 n=7+7)
RuntimeStorageWriteCached-2	83.7kB ± 0%	83.7kB ± 0%	~	(p=0.538 n=6+7)
RuntimeResourceDictionaryValues-2	4.34MB ± 0%	4.34MB ± 0%	~	(p=0.394 n=6+6)
QualifiedIdentifierCreation/One_level-2	0.00B	0.00B	~	(all equal)
QualifiedIdentifierCreation/Three_levels-2	64.0B ± 0%	64.0B ± 0%	~	(all equal)
CheckContractInterfaceFungibleTokenConformance-2	65.7kB ± 0%	65.7kB ± 0%	~	(p=0.462 n=7+7)
ContractInterfaceFungibleToken-2	26.5kB ± 0%	26.5kB ± 0%	~	(all equal)
NewInterpreter/new_interpreter-2	720B ± 0%	720B ± 0%	~	(all equal)
NewInterpreter/new_sub-interpreter-2	1.11kB ± 0%	1.11kB ± 0%	~	(all equal)
InterpretRecursionFib-2	1.24MB ± 0%	1.24MB ± 0%	~	(p=0.245 n=6+7)

	allocs/op		delta
RuntimeFungibleTokenTransfer-2	4.40k ± 0%	4.40k ± 0%	~	(p=0.462 n=7+7)
RuntimeStorageWriteCached-2	1.42k ± 0%	1.42k ± 0%	~	(all equal)
RuntimeResourceDictionaryValues-2	108k ± 0%	108k ± 0%	~	(p=0.404 n=7+7)
QualifiedIdentifierCreation/One_level-2	0.00	0.00	~	(all equal)
QualifiedIdentifierCreation/Three_levels-2	2.00 ± 0%	2.00 ± 0%	~	(all equal)
CheckContractInterfaceFungibleTokenConformance-2	1.07k ± 0%	1.07k ± 0%	~	(all equal)
ContractInterfaceFungibleToken-2	457 ± 0%	457 ± 0%	~	(all equal)
NewInterpreter/new_interpreter-2	11.0 ± 0%	11.0 ± 0%	~	(all equal)
NewInterpreter/new_sub-interpreter-2	32.0 ± 0%	32.0 ± 0%	~	(all equal)
InterpretRecursionFib-2	25.0k ± 0%	25.0k ± 0%	~	(all equal)

SupunS

Nice!

SupunS · 2021-12-03T20:41:59Z

runtime/ast/statement.go

+const forStatementSpaceInKeywordSpaceDoc = prettier.Text(" in ")
+
+func (s *ForStatement) Doc() prettier.Doc {
+	doc := prettier.Concat{


need this concat?

Yes, we always need the concatenation, as https://github.com/onflow/cadence/pull/1280/files#diff-cb6a372e592ff206bc8f5ad13e85ba6527d1e6914f281f0dd263479c7f3a278eR350 always appends more after the conditionally appended index (https://github.com/onflow/cadence/pull/1280/files#diff-cb6a372e592ff206bc8f5ad13e85ba6527d1e6914f281f0dd263479c7f3a278eR342)

SupunS · 2021-12-03T20:44:43Z

runtime/ast/statement.go

+	}
+
+	if s.Index != nil {
+		doc = append(


Is there a difference between using an array (append) vs prettier.Concat{}?

prettier.Concat is a prettier document, but is also just a slice of documents, so we can use the Go append to add to it

turbolent added 14 commits November 27, 2021 15:20

refactor block pretty printing

b8e41f6

assert each expression must conform to the expression interface

7f8a186

prettier break statement

d492ee8

prettier continue statement

38f0bca

prettier expression statement

f6c7777

prettier if statement

3d069ca

prettier while statement

3e3df9d

prettier for statement

83d673b

prettier emit statement

b877a1f

prettier assignment statement

6db9e26

prettier swap statement

13274f7

prettier switch statement

d809364

allow tabs

f6ad9f3

no position info needed in prettier doc tests

519737a

turbolent added the Feature label Nov 29, 2021

turbolent requested review from dsainati1, SupunS and robert-e-davidson3 November 29, 2021 01:25

turbolent self-assigned this Nov 29, 2021

turbolent mentioned this pull request Nov 30, 2021

Pretty print types #1281

Merged

6 tasks

Base automatically changed from bastian/prettier to master November 30, 2021 00:37

Merge branch 'master' into bastian/prettier-2

f60a37f

SupunS approved these changes Dec 3, 2021

View reviewed changes

turbolent merged commit 64592b4 into master Dec 7, 2021

turbolent deleted the bastian/prettier-2 branch December 7, 2021 10:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pretty print statements #1280

Pretty print statements #1280

turbolent commented Nov 29, 2021 •

edited

Loading

codecov-commenter commented Nov 30, 2021

github-actions bot commented Nov 30, 2021

SupunS left a comment

SupunS Dec 3, 2021 •

edited

Loading

turbolent Dec 7, 2021

SupunS Dec 3, 2021 •

edited

Loading

turbolent Dec 7, 2021

Pretty print statements #1280

Pretty print statements #1280

Conversation

turbolent commented Nov 29, 2021 • edited Loading

Description

codecov-commenter commented Nov 30, 2021

Codecov Report

github-actions bot commented Nov 30, 2021

Cadence Benchstat comparison

Results

SupunS left a comment

Choose a reason for hiding this comment

SupunS Dec 3, 2021 • edited Loading

Choose a reason for hiding this comment

turbolent Dec 7, 2021

Choose a reason for hiding this comment

SupunS Dec 3, 2021 • edited Loading

Choose a reason for hiding this comment

turbolent Dec 7, 2021

Choose a reason for hiding this comment

turbolent commented Nov 29, 2021 •

edited

Loading

SupunS Dec 3, 2021 •

edited

Loading

SupunS Dec 3, 2021 •

edited

Loading