Merge pull request #142 from serokell/Sereja313/#137-remove-checkLocalhost-option

[#137] Remove `checkLocalhost` option

CHANGES.md

@@ -41,6 +41,9 @@ Unreleased
   + Add support for glob patterns to `ignored` and `notScanned`.
   + Remove support for directory names from `ignored` and `notScanned`.
   + Fix bug with `ignored` not ignoring files with broken xrefcheck annotations.
+* [#142](https://github.com/serokell/xrefcheck/pull/142)
+  + Remove `check-localhost` CLI option and `checkLocalhost` config option.
+  + Add a regex matching localhost links to the `ignoreRefs` field of the default config.
 
 0.2.1
 ==========

links-tests/Test/Xrefcheck/FtpLinks.hs

@@ -15,7 +15,7 @@ import Test.Tasty (TestTree, askOption, testGroup)
 import Test.Tasty.HUnit (assertBool, assertFailure, testCase, (@?=))
 import Test.Tasty.Options as Tasty (IsOption (..), OptionDescription (Option), safeRead)
 
-import Xrefcheck.Config (Config (cVerification), VerifyConfig (vcCheckLocalhost), defConfig)
+import Xrefcheck.Config (Config (cVerification), VerifyConfig (vcIgnoreRefs), defConfig)
 import Xrefcheck.Core (Flavor (GitHub))
 import Xrefcheck.Verify
   (VerifyError (..), VerifyResult (VerifyResult), checkExternalResource, verifyErrors)
@@ -42,7 +42,7 @@ instance IsOption FtpHostOpt where
 
 
 config :: VerifyConfig
-config = (cVerification $ defConfig GitHub) { vcCheckLocalhost = True }
+config = (cVerification $ defConfig GitHub) { vcIgnoreRefs = [] }
 
 test_FtpLinks :: TestTree
 test_FtpLinks = askOption $ \(FtpHostOpt host) -> do

src/Xrefcheck/CLI.hs

@@ -26,7 +26,7 @@ import Data.List qualified as L
 import Data.Text qualified as T
 import Data.Version (showVersion)
 import Options.Applicative
-  (Mod, OptionFields, Parser, ReadM, auto, command, eitherReader, execParser, flag, flag',
+  (Mod, OptionFields, Parser, ReadM, auto, command, eitherReader, execParser, flag',
   footerDoc, fullDesc, help, helpDoc, helper, hsubparser, info, infoOption, long, metavar, option,
   progDesc, short, strOption, switch, value)
 import Options.Applicative.Help.Pretty (Doc, displayS, fill, fillSep, indent, renderPretty, text)
@@ -90,15 +90,13 @@ addTraversalOptions TraversalConfig{..} (TraversalOptions ignored) =
   }
 
 data VerifyOptions = VerifyOptions
-  { voCheckLocalhost :: Maybe Bool
-  , voMaxRetries     :: Maybe Int
+  { voMaxRetries     :: Maybe Int
   }
 
 addVerifyOptions :: VerifyConfig -> VerifyOptions -> VerifyConfig
-addVerifyOptions VerifyConfig{..} (VerifyOptions checkLocalhost maxRetries) =
+addVerifyOptions VerifyConfig{..} (VerifyOptions maxRetries) =
   VerifyConfig
-  { vcCheckLocalhost = fromMaybe vcCheckLocalhost checkLocalhost
-  , vcMaxRetries = fromMaybe vcMaxRetries maxRetries
+  { vcMaxRetries = fromMaybe vcMaxRetries maxRetries
   , ..
   }
 
@@ -188,9 +186,6 @@ traversalOptionsParser = do
 
 verifyOptionsParser :: Parser VerifyOptions
 verifyOptionsParser = do
-  voCheckLocalhost <- flag Nothing (Just True) $
-    long "check-localhost" <>
-    help "Check localhost links."
   voMaxRetries <- option (Just <$> auto) $
     long "retries" <>
     metavar "INT" <>

src/Xrefcheck/Config.hs

@@ -57,8 +57,6 @@ data VerifyConfig = VerifyConfig
     -- ^ Files, references in which we should not analyze.
   , vcIgnoreRefs                :: [Regex]
     -- ^ Regular expressions that match external references we should not verify.
-  , vcCheckLocalhost            :: Bool
-    -- ^ If True - we will check localhost links.
   , vcIgnoreAuthFailures        :: Bool
     -- ^ If True - links which return 403 or 401 code will be skipped,
     -- otherwise – will be marked as broken, because we can't check it.

src/Xrefcheck/Config/Default.hs

@@ -45,10 +45,9 @@ verification:
 
   # POSIX extended regular expressions that match external references
   # that have to be ignored (not verified).
-  ignoreRefs: []
-
-  # Check localhost links.
-  checkLocalhost: false
+  ignoreRefs:
+    # Ignore localhost links by default
+    - ^(https?|ftps?)://(localhost|127\.0\.0\.1).*
 
   # Skip links which return 403 or 401 code.
   ignoreAuthFailures: true

src/Xrefcheck/Verify.hs

@@ -39,7 +39,6 @@ import Control.Monad.Except (MonadError (..))
 import Data.ByteString qualified as BS
 import Data.List qualified as L
 import Data.Map qualified as M
-import Data.Text qualified as T
 import Data.Text.Metrics (damerauLevenshteinNorm)
 import Data.Time (UTCTime, defaultTimeLocale, formatTime, readPTime, rfc822DateFormat)
 import Data.Time.Clock.POSIX (getPOSIXTime)
@@ -487,7 +486,7 @@ parseUri link = do
 
 checkExternalResource :: VerifyConfig -> Text -> IO (VerifyResult VerifyError)
 checkExternalResource VerifyConfig{..} link
-  | skipCheck = return mempty
+  | isIgnored = return mempty
   | otherwise = fmap toVerifyRes $ runExceptT $ do
       uri <- parseUri link
       case toString <$> uriScheme uri of
@@ -497,10 +496,7 @@ checkExternalResource VerifyConfig{..} link
         Just "ftps" -> checkFtp uri True
         _ -> throwError ExternalResourceUnknownProtocol
   where
-    skipCheck = isIgnored || (not vcCheckLocalhost && isLocalLink)
-      where
-        isIgnored = doesMatchAnyRegex link vcIgnoreRefs
-        isLocalLink = any (`T.isInfixOf` link) ["://localhost", "://127.0.0.1"]
+    isIgnored = doesMatchAnyRegex link vcIgnoreRefs
 
     doesMatchAnyRegex :: Text -> ([Regex] -> Bool)
     doesMatchAnyRegex src = any $ \regex ->

tests/Test/Xrefcheck/ConfigSpec.hs

@@ -44,7 +44,7 @@ spec = do
               ]
 
   describe "`ignoreAuthFailures` working as expected" $ do
-    let config = (cVerification $ defConfig GitHub) { vcCheckLocalhost = True }
+    let config = (cVerification $ defConfig GitHub) { vcIgnoreRefs = [] }
 
     it "when True - assume 401 status is valid" $
       checkLinkWithServer (config { vcIgnoreAuthFailures = True })

tests/Test/Xrefcheck/TooManyRequestsSpec.hs

@@ -196,7 +196,7 @@ spec = do
     verifyReferenceWithProgress :: Reference -> IORef VerifyProgress -> IO (VerifyResult VerifyError)
     verifyReferenceWithProgress reference progRef = do
       fmap wrlItem <$> verifyReference
-        ((cVerification $ defConfig GitHub) { vcCheckLocalhost = True }) FullMode
+        ((cVerification $ defConfig GitHub) { vcIgnoreRefs = [] }) FullMode
         progRef (RepoInfo M.empty) "." "" reference
 
     -- | When called for the first time, returns with a 429 and `Retry-After: @retryAfter@`.

tests/configs/github-config.yaml

@@ -36,10 +36,9 @@ verification:
 
   # POSIX extended regular expressions that match external references
   # that have to be ignored (not verified).
-  ignoreRefs: []
-
-  # Check localhost links.
-  checkLocalhost: false
+  ignoreRefs:
+    # Ignore localhost links by default
+    - ^(https?|ftps?)://(localhost|127\.0\.0\.1).*
 
   # Skip links which return 403 or 401 code.
   ignoreAuthFailures: true

tests/golden/check-cli/config-no-scan-ignored.yaml

@@ -11,7 +11,6 @@ verification:
   notScanned: [ "to-ignore/broken-link.md" ]
   virtualFiles: []
   ignoreRefs: []
-  checkLocalhost: false
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3

tests/golden/check-ignoreRefs/check-ignoreRefs.bats

@@ -0,0 +1,37 @@
+#!/usr/bin/env bats
+
+# SPDX-FileCopyrightText: 2021 Serokell <https://serokell.io>
+#
+# SPDX-License-Identifier: MPL-2.0
+
+load '../helpers/bats-support/load'
+load '../helpers/bats-assert/load'
+load '../helpers'
+
+@test "Ignore localhost" {
+  run xrefcheck \
+    -c config-check-disabled.yaml \
+    -r .
+
+  assert_output --partial "All repository links are valid."
+}
+
+@test "Ignore localhost, check errors" {
+  xrefcheck \
+    -c config-check-enabled.yaml \
+    -r . \
+    | prepare > /tmp/check-localhost.test || true
+
+  diff /tmp/check-localhost.test expected.gold \
+    --ignore-space-change \
+    --ignore-blank-lines \
+    --new-file # treat absent files as empty
+
+  rm /tmp/check-localhost.test
+}
+
+@test "Ignore localhost, no config specified" {
+  run xrefcheck
+
+  assert_output --partial "All repository links are valid."
+}

tests/golden/check-ignoreRefs/config-check-disabled.yaml

@@ -10,8 +10,8 @@ verification:
   externalRefCheckTimeout: 10s
   notScanned: []
   virtualFiles: []
-  ignoreRefs: []
-  checkLocalhost: false
+  ignoreRefs:
+    - ^(https?|ftps?)://(localhost|127\.0\.0\.1).*
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3

tests/golden/check-ignoreRefs/config-check-enabled.yaml

@@ -11,7 +11,6 @@ verification:
   notScanned: []
   virtualFiles: []
   ignoreRefs: []
-  checkLocalhost: true
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3

tests/golden/check-ignoreRefs/expected.gold

@@ -1,6 +1,6 @@
 === Invalid references found ===
 
-  ➥  In file check-localhost.md
+  ➥  In file check-ignoreRefs.md
      bad reference (external) at src:7:10-53:
        - text: "web-site"
        - link: https://localhost:20000/web-site
@@ -10,7 +10,7 @@
 
 
 
-  ➥  In file check-localhost.md
+  ➥  In file check-ignoreRefs.md
      bad reference (external) at src:9:10-45:
        - text: "team"
        - link: https://127.0.0.1:20000/team
@@ -20,7 +20,7 @@
 
 
 
-  ➥  In file check-localhost.md
+  ➥  In file check-ignoreRefs.md
      bad reference (external) at src:11:10-44:
        - text: "blog"
        - link: http://localhost:20000/blog
@@ -30,7 +30,7 @@
 
 
 
-  ➥  In file check-localhost.md
+  ➥  In file check-ignoreRefs.md
      bad reference (external) at src:13:10-44:
        - text: "labs"
        - link: http://127.0.0.1:20000/labs

tests/golden/check-ignored/config-ignored.yaml

@@ -12,7 +12,6 @@ verification:
   notScanned: []
   virtualFiles: []
   ignoreRefs: []
-  checkLocalhost: true
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3

tests/golden/check-localhost/check-localhost.bats

@@ -1,80 +0,0 @@
-#!/usr/bin/env bats
-
-# SPDX-FileCopyrightText: 2021 Serokell <https://serokell.io>
-#
-# SPDX-License-Identifier: MPL-2.0
-
-load '../helpers/bats-support/load'
-load '../helpers/bats-assert/load'
-load '../helpers'
-
-@test "Config: check-localhost=false, CLA --check-localhost not provided" {
-  run xrefcheck \
-    -c config-check-disabled.yaml \
-    -r .
-
-  assert_output --partial "All repository links are valid."
-}
-
-@test "Config: check-localhost=false, CLA --check-localhost provided" {
-  xrefcheck \
-    -c config-check-disabled.yaml \
-    -r . \
-    --check-localhost \
-    | prepare > /tmp/check-localhost.test || true
-
-  diff /tmp/check-localhost.test expected.gold \
-    --ignore-space-change \
-    --ignore-blank-lines \
-    --new-file # treat absent files as empty
-
-  rm /tmp/check-localhost.test
-}
-
-@test "Config: check-localhost=true, CLA --check-localhost not provided" {
-  xrefcheck \
-    -c config-check-enabled.yaml \
-    -r . \
-    | prepare > /tmp/check-localhost.test || true
-
-  diff /tmp/check-localhost.test expected.gold \
-    --ignore-space-change \
-    --ignore-blank-lines \
-    --new-file # treat absent files as empty
-
-  rm /tmp/check-localhost.test
-}
-
-@test "Config: check-localhost=true, CLA --check-localhost provided" {
-  xrefcheck \
-    -c config-check-enabled.yaml \
-    -r . \
-    --check-localhost \
-    | prepare > /tmp/check-localhost.test || true
-
-  diff /tmp/check-localhost.test expected.gold \
-    --ignore-space-change \
-    --ignore-blank-lines \
-    --new-file # treat absent files as empty
-
-  rm /tmp/check-localhost.test
-}
-
-@test "Config: missing, CLA --check-localhost not provided" {
-  run xrefcheck
-
-  assert_output --partial "All repository links are valid."
-}
-
-@test "Config: missing, CLA --check-localhost provided" {
-  xrefcheck \
-    --check-localhost \
-    | prepare > /tmp/check-localhost.test || true
-
-  diff /tmp/check-localhost.test expected.gold \
-    --ignore-space-change \
-    --ignore-blank-lines \
-    --new-file # treat absent files as empty
-
-  rm /tmp/check-localhost.test
-}

tests/golden/check-notScanned/config-directory.yaml

@@ -12,7 +12,6 @@ verification:
     - notScanned/inner-directory
   virtualFiles: []
   ignoreRefs: []
-  checkLocalhost: true
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3

tests/golden/check-notScanned/config-full-path.yaml

@@ -12,7 +12,6 @@ verification:
     - ./notScanned/inner-directory/bad-reference.md
   virtualFiles: []
   ignoreRefs: []
-  checkLocalhost: true
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3

tests/golden/check-notScanned/config-nested-directories.yaml

@@ -12,7 +12,6 @@ verification:
     - ./**/*
   virtualFiles: []
   ignoreRefs: []
-  checkLocalhost: true
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3

tests/golden/check-notScanned/config-wildcard.yaml

@@ -12,7 +12,6 @@ verification:
     - ./notScanned/inner-directory/*
   virtualFiles: []
   ignoreRefs: []
-  checkLocalhost: true
   ignoreAuthFailures: true
   defaultRetryAfter: 30s
   maxRetries: 3