Skip to content
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

Maintenance: linting #123

Merged
merged 49 commits into from
May 31, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
e9b3f00
Add lint modules
samhh May 25, 2022
b7a7c56
lint module
Magellol May 25, 2022
dc82588
Add intlc-internal
samhh May 25, 2022
905281f
Tests
Magellol May 25, 2022
ad0815b
Clearer error message
Magellol May 25, 2022
e8a1588
Merge branch 'linter' of github.com:unsplash/intlc into linter
Magellol May 25, 2022
0cfd183
Use where clause
Magellol May 25, 2022
7d64d27
Get number instead of bool
Magellol May 25, 2022
6e7e84e
Rename
Magellol May 25, 2022
bd75569
Lint
Magellol May 25, 2022
9f798ea
It's anything greater than 1 to fail linting
Magellol May 25, 2022
03ed8dc
Test cases
Magellol May 25, 2022
5d86c26
Clarify
Magellol May 25, 2022
74869cd
Handle error message rendering outside of linter module (e.g in some …
Magellol May 26, 2022
5cc9666
Replace case by pattern matching inside functions
Magellol May 26, 2022
4bbf518
Use fn application
Magellol May 26, 2022
e601402
Use guard
Magellol May 26, 2022
b6238b7
Use recursion to short-circuit counting
Magellol May 26, 2022
c8d1bc9
Fix imports
Magellol May 26, 2022
95a2736
Attempt to clean it up
Magellol May 27, 2022
af9013c
Invoke linter (WIP)
Magellol May 27, 2022
360a3db
fmt
Magellol May 27, 2022
8a8cce1
Dedupe
Magellol May 27, 2022
0742643
fmt
Magellol May 27, 2022
d0310bd
More readable format
Magellol May 27, 2022
d7e0017
Fmt
Magellol May 27, 2022
f79160c
Add error label
Magellol May 27, 2022
21a0078
fmt
Magellol May 27, 2022
e03d7a0
Invert
Magellol May 27, 2022
cd03f4e
Handle already-nested cases
Magellol May 30, 2022
ef5bff3
Specify
Magellol May 30, 2022
26e5b94
Formatting
samhh May 31, 2022
3f70829
Utilise NamedFieldPuns
samhh May 31, 2022
f417059
Merge branch 'master' into linter
samhh May 31, 2022
0674840
Make lint rule deeply lazy
samhh May 31, 2022
0b640d5
Avoid mutual recursion
samhh May 31, 2022
faf2252
Remove unneeded intermediary type
samhh May 31, 2022
046dcfc
Rename to idiom "go"
samhh May 31, 2022
821e849
Rename
samhh May 31, 2022
c1445c0
tuple pattern match -> where clause
samhh May 31, 2022
1a21e1c
Hoist incrementing
samhh May 31, 2022
363fe8f
Move generic getStream fn to ICU module
samhh May 31, 2022
d258a73
Pattern match on type alone for decreased verbosity
samhh May 31, 2022
95b3f74
Merge Select pattern matches
samhh May 31, 2022
b2e1fde
Add Plural support to getStream
samhh May 31, 2022
1af6a98
Don't print anything upon no lint errors
samhh May 31, 2022
40a7610
Import qualified
samhh May 31, 2022
76fb999
Don't show/print status type
samhh May 31, 2022
a56710e
Support multiple lint rules
samhh May 31, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions internal/CLI.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{-# OPTIONS_GHC -Wno-unrecognised-pragmas #-}
{-# HLINT ignore "Use newtype instead of data" #-}

module CLI (Opts (..), getOpts) where

import Options.Applicative
import Prelude

data Opts
= Lint FilePath

getOpts :: IO Opts
getOpts = execParser (info (opts <**> helper) (progDesc h))
where h = "Additional tooling for Unsplash."

opts :: Parser Opts
opts = subparser
( command "lint" (info (lint <**> helper) mempty)
)

lint :: Parser Opts
lint = Lint <$> pathp

pathp :: Parser FilePath
pathp = argument str (metavar "filepath")
9 changes: 9 additions & 0 deletions internal/Main.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Main where

import CLI (Opts (..), getOpts)
import qualified Data.Text as T
import Prelude

main :: IO ()
main = getOpts >>= \case
Lint x -> putTextLn $ "Hello, " <> T.pack x <> "!"
12 changes: 12 additions & 0 deletions intlc.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ executable intlc
other-modules:
CLI

executable intlc-internal
import: common
hs-source-dirs: internal/
main-is: Main.hs
build-depends:
intlc
, optparse-applicative ^>=0.16
other-modules:
CLI

library
import: common
hs-source-dirs: lib/
Expand All @@ -54,6 +64,7 @@ library
Intlc.Backend.TypeScript.Compiler
Intlc.Core
Intlc.ICU
Intlc.Linter
Intlc.Parser
Intlc.Parser.Error
Intlc.Parser.JSON
Expand Down Expand Up @@ -81,5 +92,6 @@ test-suite test-intlc
Intlc.Backend.TypeScriptSpec
Intlc.CompilerSpec
Intlc.EndToEndSpec
Intlc.LinterSpec
Intlc.Parser.JSONSpec
Intlc.Parser.ICUSpec
37 changes: 37 additions & 0 deletions lib/Intlc/Linter.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
module Intlc.Linter where

import Intlc.ICU
import Prelude hiding (Type)

data Status
= Success
| Failure Text
deriving (Eq, Show)

countInterpolations :: NEStream -> Int
countInterpolations = foldr go 0
Magellol marked this conversation as resolved.
Show resolved Hide resolved
where
count = \case
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I count three pattern matches with case that could be inlined into their functions. 😛 e.g.

-- before
f x = case x of y -> z
-- or
f = \case y -> z

-- after
f y = z

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found these two helpful to change 5cc9666

I'm guessing the third one is count = \case although if the equivalent would be

count String = 0
count Number = 0
count (Bool {}) = 1

Would this be very similar to using point-free case here? Or perhaps there is another equivalent I'm not seeing 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's the one, and yeah it's very similar, just a case of common idioms. 🙂

String -> 0
Number -> 0
Date {} -> 0
Time {} -> 0
PluralRef -> 0
Bool {} -> 1
Plural {} -> 1
Select {} -> 1
Callback {} -> 1
go :: Token -> Int -> Int
go t n = case t of
Plaintext {} -> n
Interpolation ((Arg _ x)) -> count x + n
Magellol marked this conversation as resolved.
Show resolved Hide resolved

lint :: Message -> Status
lint m = case m of
Static {} -> Success
Magellol marked this conversation as resolved.
Show resolved Hide resolved
Dynamic neStream -> (mkStatus . countInterpolations) neStream
samhh marked this conversation as resolved.
Show resolved Hide resolved
where
mkStatus n = if n > 1
samhh marked this conversation as resolved.
Show resolved Hide resolved
then Failure "Too many interpolations. They will appear nested once flattened."
else Success

30 changes: 30 additions & 0 deletions test/Intlc/LinterSpec.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
module Intlc.LinterSpec where

import Intlc.ICU
import Intlc.Linter
import Prelude
import Test.Hspec

spec :: Spec
spec = describe "linter" $ do
it "lints static text" $ do
lint (Static "Hello world") `shouldBe` Success

it "lints dynamic streams with 1 plain text token" $ do
lint (Dynamic (fromList [Plaintext "yay"])) `shouldBe` Success

it "lints dynamic streams with 2 or more plain text token" $ do
lint (Dynamic (fromList [Plaintext "yay", Plaintext "Hello"])) `shouldBe` Success

it "lints dynamic streams with 1 dynamic simple interpolation" $ do
lint (Dynamic (fromList [Interpolation (Arg "Hello" String)])) `shouldBe` Success

it "lints dynamic streams with 1 dynamic complex interpolation" $ do
lint (Dynamic (fromList [Interpolation (Arg "Hello" (Callback []))])) `shouldBe` Success

it "lints dynamic streams with 1 dynamic complex interpolation and 1 simple interpolation" $ do
lint (Dynamic (fromList [Interpolation (Arg "Hello" (Callback [])), Plaintext "hello"])) `shouldBe` Success

it "does not lint dynamic streams with 2 or more dynamic complex interpolations" $ do
lint (Dynamic (fromList [Interpolation (Arg "Hello" (Callback [])), Interpolation (Arg "Hello" (Bool [] []))])) `shouldBe` Failure "Too many interpolations. They will appear nested once flattened."
Magellol marked this conversation as resolved.
Show resolved Hide resolved