diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 5d33b1e2b25..a1adda3d5d6 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -14,6 +14,6 @@ Please include the following checklist in your PR: - [ ] The documentation has been updated if necessary. -- [ ] The code conforms to the [Scala](https://github.com/luna/enso/blob/master/doc/scala-style-guide.md) or [Haskell](https://github.com/luna/enso/blob/master/doc/haskell-style-guide.md) style guides as appropriate. +- [ ] The code conforms to the [Scala](https://github.com/luna/enso/blob/master/doc/scala-style-guide.md), [Java](https://github.com/luna/enso/blob/master/doc/java-style-guide.md) or [Haskell](https://github.com/luna/enso/blob/master/doc/haskell-style-guide.md) style guides as appropriate. - [ ] The code has been tested where possible. diff --git a/doc/haskell-style-guide.md b/doc/haskell-style-guide.md index 25c2980111b..f1e9b17e3de 100644 --- a/doc/haskell-style-guide.md +++ b/doc/haskell-style-guide.md @@ -1,7 +1,7 @@ # Haskell Style Guide Like many style guides, this Haskell style guide exists for two primary reasons. The first is to provide guidelines that result in a consistent code style across -all of the Luna codebases, while the second is to guide people towards a style +all of the Enso codebases, while the second is to guide people towards a style that is expressive while still easy to read and understand. In general, it aims to create a set of 'zero-thought' rules in order to ease the @@ -42,7 +42,7 @@ This section explains the rules for visually laying out your code. They provide a robust set of guidelines for creating a consistent visual to the code. ### Whitespace -The rules for whitespace in the Luna codebases are relatively simple: +The rules for whitespace in the Enso codebases are relatively simple: - 4 spaces are used for indentation, with no tabs. - There should not be any trailing whitespace. @@ -145,7 +145,7 @@ multiple lines, it _no longer counts_ as visually similar, and hence subsequent lines should not be aligned with it. ### Naming -Luna has some fairly simple general naming conventions, though the sections +Enso has some fairly simple general naming conventions, though the sections below may provide more rules for use in specific cases. - Types are written using `UpperCamelCase`. @@ -183,9 +183,9 @@ module unqualified. This can be seen with `Map` in the examples below. This example is for a module that re-exports some names: ```hs -module Luna.MyModule (module Luna.MyModule, module X) where +module Enso.MyModule (module Enso.MyModule, module X) where -import Luna.MyModule.Class as X (foo, bar) +import Enso.MyModule.Class as X (foo, bar) import Prologue @@ -201,7 +201,7 @@ However, in the context where your module doesn't re-export anything, you can use the simplified form: ```hs -module Luna.MyModule where +module Enso.MyModule where import Prologue @@ -219,8 +219,8 @@ module that hasn't been exported. To that end, we do not allow for restricted export lists in our modules. Instead, if you want to indicate that something is for internal use, you need to -define it in an internal module. For a module named `Luna.MyModule`, we can -define internal functions and data-types in `Luna.MyModule.Internal`. This means +define it in an internal module. For a module named `Enso.MyModule`, we can +define internal functions and data-types in `Enso.MyModule.Internal`. This means that these functions can be imported by clients of the API if they need to, but that we provide no guarantees about API stability when using those functions. @@ -277,7 +277,7 @@ Code should be written in such a way that it guides you over what it does, and comments should not be used as a crutch for badly-designed code. ### Documentation Comments -One of the primary forms of comment that we allow across the Luna codebases is +One of the primary forms of comment that we allow across the Enso codebases is the doc comment. These are intended to be consumed by users of the API, and use the standard Haddock syntax. Doc comments should: @@ -392,12 +392,12 @@ Any good style guide goes beyond purely stylistic rules, and also talks about design styles to use in code. ### Libraries -The Luna project has many internal libraries that are useful, but we have found +The Enso project has many internal libraries that are useful, but we have found that maintaining these on Hackage while they are under such active development is counterproductive. Instead, libraries live in the `lib/` folder of the primary project with which -they are associated (Luna, Luna Studio, or Dataframes). These libraries may be +they are associated (Enso, Enso Studio, or Dataframes). These libraries may be freely used by others of our projects by depending on a git commit of the project that they live in. All of these are safe to use. @@ -438,7 +438,7 @@ We have our own exception framework based on `ExceptT` that encodes exception usage at the type level. This ensures that all synchronous exceptions must be dealt with. -It is defined in [`lib/exception/`](https://github.com/luna/luna/tree/master/lib/exception) +It is defined in [`lib/exception/`](https://github.com/Enso/Enso/tree/master/lib/exception) and contains utilities for declaring that a function throws an exception, as well as throwing and catching exceptions. @@ -468,14 +468,14 @@ in mind: - When designing a module that exports a type, the module should be named after that type. If it exports multiple types, there should be a primary type, or the other types should be factored out into their own modules. -- We import modules as their name. If you have a module `Luna.Space.MyType`, we +- We import modules as their name. If you have a module `Enso.Space.MyType`, we import it qualified as `MyType`. - Functions should be named with the assumption of being used qualified. This means that we rarely refer to the module name in the function name (e.g. `State.run` rather than `State.runState`). ### Data Declarations -When declaring data types in the Luna codebases, please make sure to keep the +When declaring data types in the Enso codebases, please make sure to keep the following rules of thumb in mind: - For single-constructor types: @@ -515,7 +515,7 @@ following rules of thumb in mind: + Monad Transformers: `MonadTrans`. #### Lenses -The Luna codebases make significant use of Lenses, and so we have some rules for +The Enso codebases make significant use of Lenses, and so we have some rules for their use: - Always use the `makeLenses` wrapper exported from `Prologue`. @@ -556,7 +556,7 @@ in a rigorous fashion. run, and should use the mechanisms HSpec provides for automatic test discovery. - A test file should be named after the module it tests. If the module is named - `Luna.MyModule`, then the test file should be named `Luna.MyModuleSpec`. + `Enso.MyModule`, then the test file should be named `Enso.MyModuleSpec`. Any performance-critical code should also be accompanied by a set of benchmarks. These are intended to allow us to catch performance regressions as the code @@ -598,7 +598,7 @@ of `foo $ bar $ baz $ bam quux`, you should write `foo . bar. baz $ bam quux` to use function composition. ## Language Extensions -Much like any sophisticated Haskell codebase, Luna makes heavy use of the GHC +Much like any sophisticated Haskell codebase, Enso makes heavy use of the GHC language extensions. We have a broad swath of extensions that are enabled by default across our projects, and a further set which are allowed whenever necessary. We also have a set of extensions that are allowed with care, which @@ -616,9 +616,9 @@ extension (linked from the extension's table below). ### Default Extensions The following language extensions are considered to be so safe, or to have such -high utility, that they are considered to be Luna's set of default extensions. -You can find said set of extensions for Luna itself defined in a -[common configuration file](https://github.com/luna/luna/blob/master/config/hpack-common.yaml). +high utility, that they are considered to be Enso's set of default extensions. +You can find said set of extensions for Enso itself defined in a +[common configuration file](https://github.com/Enso/Enso/blob/master/config/hpack-common.yaml). #### AllowAmbiguousTypes @@ -698,7 +698,7 @@ are promoted to kinds and the value constructors are promoted to type constructors. This is incredibly useful, and used heavily in the type-level programming that -makes the Luna codebase so expressive and yet so safe. +makes the Enso codebase so expressive and yet so safe. #### DefaultSignatures @@ -1365,7 +1365,7 @@ cause the typechecker to loop. ### Disallowed Extensions If a language extension hasn't been listed in the above sections, then it is -considered to be disallowed throughout the Luna codebases. If you have a good +considered to be disallowed throughout the Enso codebases. If you have a good reason to want to use one of these disallowed extensions, please talk to Ara or Wojciech to discuss its usage. diff --git a/doc/java-style-guide.md b/doc/java-style-guide.md new file mode 100644 index 00000000000..41e0e75994f --- /dev/null +++ b/doc/java-style-guide.md @@ -0,0 +1,223 @@ +# Java Style Guide +Like many style guides, this Java style guide exists for two primary reasons. +The first is to provide guidelines that result in a consistent code style across +all of the Enso codebases, while the second is to guide people towards a style +that is expressive while still easy to read and understand. + +In general, it aims to create a set of 'zero-thought' rules in order to ease the +programmer burden; there is usually only _one way_ to lay out code correctly. + + + +- [Code Formatting](#code-formatting) + - [Naming](#naming) +- [Commenting](#commenting) + - [Documentation Comments](#documentation-comments) + - [Source Notes](#source-notes) + - [TODO Comments](#todo-comments) + - [Other Comment Usage](#other-comment-usage) +- [Program Design](#program-design) + - [Testing and Benchmarking](#testing-and-benchmarking) + - [Warnings, and Lints](#warnings-and-lints) + + + +## Code Formatting +This section explains the rules for visually laying out your code. They provide +a robust set of guidelines for creating a consistent visual to the code. + +Primary code formatting is done using the [Google Java Format](https://github.com/google/google-java-format) +tool, which enforces a clear and consistent style. This is a zero configuration +tool, and hence there is no project-level configuration for this tool. It should +be used for all new Java projects. + +All files must be formatted using this tool before being committed, and this +should be set up as either a precommit hook, or using an integration in your +IDE. + +### Naming +Enso has some fairly simple general naming conventions, though the sections +below may provide more rules for use in specific cases. + +- Types are written using `UpperCamelCase`. +- Variables and function names are written using `camelCase`. +- If a name contains an initialism or acronym, all parts of that initialism + should be of the same case: `httpRequest` or `makeHTTPRequest`. +- Short variable names such as `a` and `b` should only be used in contexts where + there is no other appropriate name, and should _never_ be used to refer to + temporary data in a function. +- Names should be descriptive, even if this makes them longer. + +## Commenting +Comments are a tricky area to get right, as we have found that comments often +expire quickly and, in absence of a way to validate them, remain incorrect for +long periods of time. That is not to say, however, that we eschew comments +entirely. Instead, we make keeping comments up to date an integral part of our +programming practice, while also limiting the types of comments that we allow. + +When we write comments, we try to follow one general guideline. A comment should +explain _what_ and _why_, without mentioning _how_. The _how_ should be +self-explanatory from reading the code, and if you find that it is not, that is +a sign that the code in question needs refactoring. + +Code should be written in such a way that it guides you over what it does, and +comments should not be used as a crutch for badly-designed code. + +### Documentation Comments +One of the primary forms of comment that we allow across the Enso codebases is +the doc comment. These are intended to be consumed by users of the API, and use +the standard Javadoc syntax. Doc comments should: + +- Provide a short one-line explanation of the object being documented. +- Provide a longer description of the object, including examples where relevant. +- Explain the arguments to a function where relevant. + +They should not reference internal implementation details, or be used to explain +choices made in the function's implementation. See [Source Notes](#source-notes) +below for how to indicate that kind of information. + +### Source Notes +Source Notes is a mechanism for moving detailed design information about a piece +of code out of the code itself. In doing so, it retains the key information +about the design while not impeding the flow of the code. + +Source notes are detailed comments that, like all comments, explain both the +_what_ and the _why_ of the code being described. In very rare cases, it may +include some _how_, but only to refer to why a particular method was chosen to +achieve the goals in question. + +A source note comment is broken into two parts: + +1. **Referrer:** This is a small comment left at the point where the explanation + is relevant. It takes the following form: `// Note [Note Name]`, where + `Note Name` is a unique identifier across the codebase. These names should be + descriptive, and make sure you search for it before using it, in case it is + already in use. +2. **Source Note:** This is the comment itself, which is a large block comment + placed after the first function in which it is referred to in the module. It + uses the java block-comment syntax `/* ... */`, and the first line names + the note using the same referrer as above: `/* Note [Note Name]`. The name(s) + in the note are underlined using a string of the `~` (tilde) character. + +A source note may contain sections within it where necessary. These are titled +using the following syntax: `== Note [Note Name (Section Name)]`, and can be +referred to from a referrer much as the main source note can be. + +Sometimes it is necessary to reference a source note in another module, but this +should never be done in-line. Instead, a piece of code should reference a source +note in the same module that references the other note while providing +additional context to that reference. + +An example, based on some code in the GHC codebase, can be seen below: + +```java +{ +public SimplM prepRHS(SimplEnv env, OutExpr outExpr) { + var ty1 = coercionKind(env); // Note [Float Coercions] + + if (!isUnliftedType(ty1)) { + var newTy1 = convertTy(ty1) // Note [Float Coercions (Unlifted)] + + ...more code defining prepRHS... + } +} + +/* Note [Float Coercions] +~~~~~~~~~~~~~~~~~~~~~~~~~ +When we find the binding + x = cast(e, co) +we'd like to transform it to + x' = e + x = cast(x, co) // A trivial binding +There's a chance that e will be a constructor application or function, or +something like that, so moving the coercion to the usage site may well cancel +the coercions and lead to further optimisation. + ...more stuff about coercion floating... + +== Note [Float Coercions (Unlifted)] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ...explanations of floating for unlifted types... +*/ +} +``` + +A source note like this is useful whenever you have design decisions to explain, +but can also be used for: + +- **Formulae and Algorithms:** If your code makes use of a mathematical formula, + or algorithm, it should note where the design element came from, preferably + with a link. +- **Safety:** Sometimes it is necessary to use an unsafe API in a context where + it is trivially made safe. You should always use a source note to explain why + its usage is safe in this context. + +### TODO Comments +We follow a simple convention for `TODO` comments in our codebases: + +- The line starts with `TODO` or `FIXME`. +- It is then followed by the author's initials `[ARA]`, or for multiple people + `[ARA, WD]`, in square brackets. +- It is then followed by an explanation of what needs to be done. + +For example: + +```java +{ +// TODO [ARA] This is a bit of a kludge. Instead of X it should to Y, accounting +// for the fact that Z. +} +``` + +### Other Comment Usage +There are, of course, a few other situations where commenting is very useful: + +- **Commenting Out:** You may comment out code while developing it, but if you + commit any commented out code, it should be accompanied by an explanation of + why said code can't just be deleted. +- **Bugs:** You can use comments to indicate bugs in our code, as well as + third-party bugs. In both cases, the comment should link to the issue tracker + where the bug has been reported. + +## Program Design +Any good style guide goes beyond purely stylistic rules, and also talks about +design styles to use in code. + +### Testing and Benchmarking +New code should always be accompanied by tests. These can be unit, integration, +or some combination of the two, and they should always aim to test the new code +in a rigorous fashion. + +- We tend to use ScalaTest, but also make use of ScalaCheck for property-based + testing. +- Tests should be declared in the project configuration so they can be trivially + run. +- A test file should be named after the module it tests. + +Any performance-critical code should also be accompanied by a set of benchmarks. +These are intended to allow us to catch performance regressions as the code +evolves, but also ensure that we have some idea of the code's performance in +general. + +- We use Caliper for our benchmarks. +- We measure time, but also memory usage and CPU time where possible. +- Where relevant, benchmarks may set thresholds which, when surpassed, cause the + benchmark to fail. These thresholds should be set for a release build, and not + for a development build. + +_Do not benchmark a development build_ as the data you get will often be +entirely useless. + +### Warnings, and Lints +In general, we aim for a codebase that is free of warnings and lints, and we do +this using the following ideas. + +#### Warnings +New code should introduce no new warnings onto master. You may build with +warnings on your own branch, but the code that is submitted as part of a PR +should not introduce new warnings. You should also endeavour to fix any warnings +that you come across during development. + +Sometimes it is impossible to fix a warning (often in situations involving the +use of macros or code-generation). In such cases, you are allowed to suppress +the warning locally, but this must be accompanied by a source note explaining +why you are doing so. diff --git a/doc/scala-style-guide.md b/doc/scala-style-guide.md index ca6e4ddfd24..a77e387c2ed 100644 --- a/doc/scala-style-guide.md +++ b/doc/scala-style-guide.md @@ -1,7 +1,7 @@ # Scala Style Guide Like many style guides, this Scala style guide exists for two primary reasons. The first is to provide guidelines that result in a consistent code style across -all of the Luna codebases, while the second is to guide people towards a style +all of the Enso codebases, while the second is to guide people towards a style that is expressive while still easy to read and understand. In general, it aims to create a set of 'zero-thought' rules in order to ease the @@ -30,7 +30,7 @@ a robust set of guidelines for creating a consistent visual to the code. Primary formatting is dealt with through use of the Scala formatting tool [`scalafmt`](https://scalameta.org/scalafmt/), which enforces rules around -whitespace, line-wrapping, and alignment. The Luna repository contains the main +whitespace, line-wrapping, and alignment. The Enso repository contains the main [`.scalafmt.conf`](../.scalafmt.conf) configuration file, and this is what should be used for all new Scala projects. @@ -42,7 +42,7 @@ installed, and be using IntelliJ 2019.1 or later. You should _not_ use the independent Scalafmt plugin. ### Naming -Luna has some fairly simple general naming conventions, though the sections +Enso has some fairly simple general naming conventions, though the sections below may provide more rules for use in specific cases. - Types are written using `UpperCamelCase`. @@ -55,7 +55,7 @@ below may provide more rules for use in specific cases. - Names should be descriptive, even if this makes them longer. ## Build Tooling -All Scala projects in the Luna organisation should manage their dependencies and +All Scala projects in the Enso organisation should manage their dependencies and build setup using [SBT](hhttps://www.scala-sbt.org/1.x/docs/index.html). If you are using IntelliJ, please ensure that you select to use the SBT shell @@ -77,7 +77,7 @@ Code should be written in such a way that it guides you over what it does, and comments should not be used as a crutch for badly-designed code. ### Documentation Comments -One of the primary forms of comment that we allow across the Luna codebases is +One of the primary forms of comment that we allow across the Enso codebases is the doc comment. These are intended to be consumed by users of the API, and use the standard [scaladoc](https://docs.scala-lang.org/style/scaladoc.html) syntax. Doc comments should: @@ -126,14 +126,13 @@ An example, based on some code in the GHC codebase, can be seen below: ```scala { -def prepRHS (env : SimplEnv, outExpr : OutExpr) : SimplM (SimplEnv, OutExpr) = { - (ty1, _ty2) <- coercionKind env // Note [Float Coercions] +def prepRHS (env : SimplEnv, outExpr : OutExpr) : SimplM[SimplEnv, OutExpr] = { + val (ty1, _ty2) = coercionKind(env) // Note [Float Coercions] - if (!isUnliftedType ty1) { - newTy1 = convertTy ty1 // Note [Float Coercions (Unlifted)] - - ...more expressions defining prepRHS... - } + if (!isUnliftedType(ty1)) { + val newTy1 = convertTy(ty1) // Note [Float Coercions (Unlifted)] + ...more expressions defining prepRHS... + } } /* Note [Float Coercions] @@ -248,4 +247,4 @@ that you come across during development. Sometimes it is impossible to fix a warning (often in situations involving the use of macros). In such cases, you are allowed to suppress the warning locally, -but this must be accompanied by a source note explaining why. +but this must be accompanied by a source note explaining why you are doing so.