@@ -6,15 +6,18 @@
66 * [Stylish Haskell](#stylish-haskell)
77 * [Haskell formatter](#haskell-formatter)
88* [Proposed design](#proposed-design)
9− * [Parsing and CPP](#parsing-and-cpp)
9+ * [Parsing](#parsing)
10+ * [CPP](#cpp)
1011 * [Printing](#printing)
1112 * [Configuration](#configuration)
1213 * [Handling of language extensions](#handling-of-language-extensions)
1314 * [Testing](#testing)
15+ * [Functionality of executable](#functionality-of-executable)
16+ * [Why not contribute to/fork Hindent or Brittany?](#why-not-contribute-to-fork-hindent-or-brittany)
1417* [Roadmap](#roadmap)
1518
1619This document describes design of a new formatter for Haskell source code.
17−It also includes some recommendations for future implementers.
20+It also includes recommendations for future implementers.
1821
1922## Analysis of the existing solutions
2023
@@ -33,7 +36,8 @@ code, but also with CPP. However it does so by running the preprocessor with
3336parameters provided by user (such as values of `#defines`). It doesn't parse
3437actual preprocessor directives in a way that would allow their
3538reconstruction, thus Brittany still lacks support for handling source code
36−with CPP in it. Which renders it almost unusable straight away.
39+with CPP in it. That said, we'll show later supporting CPP correctly is
40+virtually impossible anyway.
3741
3842After parsing, Haskell AST and a collection of annotations are available.
3943The annotations are there because Haskell AST doesn't provide enough
@@ -50,8 +54,7 @@ The code is hard to read and is buggy too, as one would expect with such
5054approach. There are enough bugs in Brittany at the moment so that it's
5155hardly usable although it's now 2 years old. Looking at the opened bugs it's
5256clear that almost all of them are because of the too-low-level approach
53−which seems to be very fragile. I'm surprised it even works at all, probably
54−the author had strong will power.
57+which seems to be very fragile.
5558
5659### Hindent
5760
@@ -71,9 +74,9 @@ about pretty-printing parsed Haskell code.
7174Perhaps surprisingly, this approach seems to be better (IMO), for several
7275reasons:
7376
74−1. Brittany cannot guarantee that the output will be “canoncial”. With given
75− config, if we pass it different inputs expressing identical Haskell
76− programs modulo spacing and indentation, by design it may produce
77+1. Brittany cannot guarantee that the output will be “canonical”. With given
78+ configuration, if we pass it different inputs expressing identical
79+ Haskell programs modulo spacing and indentation, by design it may produce
7780 different outputs:
7881
7982 ```
@@ -96,13 +99,13 @@ reasons:
9699 canonical output
97100 ```
98101
99− Because there is one way to pretty print a parsed program. This means
100− that the users won't need to think about the layout at all because it'll
101− be 100% determined by the pretty-printer after the transformation.
102+ Because there is one way to pretty print parsed program. This means that
103+ the users won't need to think about the layout at all because it'll be
104+ 100% determined by the pretty-printer after the transformation.
102105
1031062. The code is easier to read and debug. Pretty-printing functions are very
104107 straightforward. If there is a bug (in pretty-printer, not in parser
105− which Hindent cannot control), it's easy to fix AFAIK.
108+ which Hindent cannot control), it's easy to fix AFAIU.
106109
107110Hindent is also notable for its ability to handle CPP and inputs that do not
108111constitute complete modules. It splits input stream into so-called “code
@@ -143,8 +146,8 @@ the issues are about upstream problems with `haskell-src-exts`.
143146
144147## Proposed design
145148
146−In this section I describe a solution that combines all the good things from
147−the ones above and tries to solve the CPP problem better.
149+This section describes a solution that combines all the good things from the
150+projects above.
148151
149152### Parsing and CPP
150153
@@ -156,28 +159,93 @@ it's better to use the parser for modules because it should work with all
156159input files containing complete modules, while with other parsers it's
157160impossible to guess what they'll be called on.
158161
159−So far so good, but CPP problem remains. We cannot ignore it as does Stylish
160−Haskell, and cannot error out on CPP as does Brittany, and Hident solves the
161−problem only partially and we cannot follow that path because
162−`ghc-exactprint` won't accept anything but a complete module, as noted
163−above.
164−
165−One workaround is to do the following:
166−
167−1. Replace CPP with comments containing first some magic word and then the
168− actual CPP directive.
169−2. Run GHC parser on that as usual. That will succeed and preserve the
170− comments in annotations.
171−3. When printing, detect these special comments and output them as CPP
172− directives again.
173−
174−The approach above may present some difficulties though:
175−
176−* When conditional CPP directives are turned into comments it's likely that
177− we'll end up with invalid Haskell if code from several conditional
178− branches is present at the same time.
179−* Restoring CPP properly and inserting it back may be triciier than we
180− think.
162+### CPP
163+
164+CPP problem remains and probably is unsolvable because:
165+
166+* GHC parser won't accept anything but a valid, complete module.
167+
168+* To preserve CPP directives they must be either removed from input and
169+ saved in some sort of structure or turned into a special sort of comments
170+ and restored later, but that is not easy because then the formatting logic
171+ itself should somehow be aware of CPP directives (mainly conditiionals)
172+ and avoid performing transformations that may lead to incorrect Haskell in
173+ the end. This is much, much harder than simple pretty-printing we want to
174+ do.
175+
176+Here is example that proves that CPP is hard to support if at all possible.
177+Suppose
178+
179+```haskell
180+f = f1
181+ where
182+ f1 = g
183+#if C1
184+g = g1
185+ where
186+ g1 = g2
187+ where
188+ g2 = False
189+#else
190+ g = True
191+#end
192+```
193+
194+After commenting CPP directives:
195+
196+```haskell
197+f = f1
198+ where
199+ f1 = g
200+{-[#if C1]-}
201+g = g1
202+ where
203+ g1 = g2
204+ where
205+ g2 = False
206+{-[#else]-}
207+ g = True
208+{-[#end]-}
209+```
210+
211+After formatting:
212+
213+```haskell
214+f = f1
215+ where
216+ f1 = g
217+{-[#if C1]-}
218+g = g1
219+ where
220+ g1 = g2
221+ where
222+ g2 = False
223+{-[#else]-}
224+ g = True
225+{-[#end]-}
226+```
227+
228+Output after uncommenting CPP directives:
229+
230+```haskell
231+f = f1
232+ where
233+ f1 = g
234+#if C1
235+g = g1
236+ where
237+ g1 = g2
238+ where
239+ g2 = False
240+#else
241+ g = True
242+#end
243+```
244+
245+Now the definition of `f` is broken when `C1` doesn't hold.
246+
247+Thus CPP should not be supported and if CPP extension is enabled we should
248+signal an error right away.
181249
182250### Printing
183251
@@ -222,49 +290,79 @@ pretty-printing code and new issues are discovered. The logic is
222290straightforward:
223291
2242921. Given input snippet of source code parse it and pretty print.
225−2. Check the output against expected output. Thus all tests should include
293+2. Parse the result of pretty-printing again and make sure that AST is the
294+ same as AST of original snippet module span positions.
295+3. Check the output against expected output. Thus all tests should include
226296 two files: input and expected output.
227−3. Check that running the formatter on the output produces the same output
228− again (the transformation is idempotent). This also checks that the
229− output is still acceptable for the GHC parser.
230−4. It's a good idea to steal test cases from test suites of existing
297+4. Check that running the formatter on the output produces the same output
298+ again (the transformation is idempotent).
299+5. It's a good idea to steal test cases from test suites of existing
231300 libraries like Brittany and Hindent.
232−5. After that we may add test cases for opened issues that Brittany and
301+6. After that we may add test cases for opened issues that Brittany and
233302 Hindent have.
234−6. When we're confident enough we can start “mining” new issues by running
303+7. When we're confident enough we can start “mining” new issues by running
235304 the program on real source code from Hackage till we don't get new issues
236305 anymore. For every issue that we find this way a test case should be
237306 added.
238307
308+### Functionality of executable
309+
310+* In all cases the program should test if the produced AST is the same as
311+ the one we originally parsed and if it differs, an error should be
312+ displayed suggesting reporting this on our issue tracker.
313+* Check mode: return non-zero exit code if any transformations would be
314+ applied.
315+* Modification in place and printing of formatted code to stdout.
316+* A flag for version/commit information.
317+* An option to specify location of config file.
318+* Options to specify parameters that come from config files on command line
319+ instead, for example indent levels, maximal line width, etc.
320+
321+### Why not contribute to/fork HIndent or Brittany?
322+
323+* Forking or contributing to Brittany is not a good idea because this would
324+ require re-doing of all transformation logic, which is harder than writing
325+ pretty-printing code from scratch. Good documentation would help
326+ readability of the code somewhat, but then we would need to spend time
327+ either collaborating with the original author or investigating how
328+ everything works ourselves.
329+
330+ Of course with sufficient persistence we could succeed in fixing
331+ Brittany's bugs, but the point is that pretty-printing à la Hindent is
332+ more maintainable (IMO) and switching to that is equal to re-doing the
333+ project.
334+
335+* Forking or contributing to Hindent is not an option because if we replace
336+ `haskell-src-exts` with `ghc` (or `ghc-exact-print`) then we'll have to
337+ work with a different AST type and all the code in Hindent will become
338+ incompatible and there won't be much code to be re-used in that case. It
339+ is also possible that we'll find a nicer way to write pretty-printer.
340+
239341## Roadmap
240342
241−Proposed roadmap (for a single person, about 38 full-time work days):
343+Proposed roadmap (for a single person, about 39 full-time work days):
242344
243−* Create and setup a repo, setup CI (less than 1 day).
244−* Implement parsing, for now without CPP handling (1 day).
345+* ~~Create and setup a repo, setup CI (less than 1 day).~~
346+* Implement parsing (2 days).
245347* Implement basis for pretty-printing of code. We could take
246348 [this][hindent-printer] as inspiration, although AST from the `ghc`
247349 package may be slightly different. Also, don't forget about annotations
248350 and comments (1 week).
249351* Implement executable program so we can easier fool around. Try it on
250− simple code samples (1 day).
352+ simple code samples (2 days).
353+* Implement location and reading of configuration file in YAML format (2
354+ days).
251355* Continue writing the pretty-printing code till we cover everything. Start
252356 adding tests, see [the section about testing](#testing). Fix bugs. At this
253357 point just repeat this till we cover everything that Brittany and Hindent
254358 can do maybe by stealing their tests (at least 2 weeks).
255−* Add support for CPP as described above. Also add the corresponding tests.
256− Check that we're doing OK in the cases that are described in opened issues
257− on the issue trackers of Brittany/Hindent/Stylish Haskell (1 week).
258359* Start checking how our formatter works on real-world code from Hackage.
259360 Fix bugs that we find that way (2 weeks).
260361* Write a good readme explaining why the project was created and how it is
261− better, etc (1 day).
262−* Make and announce the first public release on Twitter/Reddit (less than 1
263− day).
264−* Improve the executable by adding more interesting options, see other
265− projects for inspiration. For example, it would be good to have an option
266− which could check if running the formatter would change anything (1-3
267− days).
362+ better, etc (2 days).
363+* Improve the executable by adding more interesting options (2 days).
364+* Switch CI to Travis and test with multiple GHC versions (1 day).
365+* Make and announce the first public release on Twitter/Reddit (1 day).
268366* Figure out how to use the formatter application with major text editors
269367 and add the info to the readme (no estimate).
270368