I believe you want advanceSrcLoc (srcSpanStart (getLoc $2)) '!' == srcSpanStart (getLoc $3) here. Note that $2 binds the qop (which could be the bang) and $3 (which corresponds to x in the test case) binds exp10_top.

Note that advanceSrcLoc <...> '!' doesn't test whether $2 was a bang, so this would still catch all single-character operators like f + x = .... It should probably also test that $2 is indeed a bang.

And if I'm not mistaken, this should really catch whitespace: If $3 doesn't immediately come after $2, then there must be some kind of separating whitespace. Otherwise there would be some other token in-between, hence the test.

Also, I don't think we even need advanceSrcLoc (srcSpanStart (getLoc $2)), becuase it's basically equivalent to stcSpanEnd (getLoc $2), which makes the test even simpler and more understandable. If that's not enough, we could give it a descriptive name, like noSeparatingWhitespace.

Ultimately, we should probably think of a better message than what this generates (e.g. "Did you forget to activate -XBangPatterns? If not, add a space before the bang").

This should also have the case where we don't want to emit a warning, e.g.

f4 :: [a] -> Int -> a
f4 ! x = f4 Prelude.! x

And also some tests for local let bindings, e.g.

a :: IO Int
a =
   let f4 !x = return (sum x)
   in f4 [1,2,3]

a :: Int
a =
  let f4 ! x = f4 Prelude.! x
  in [1,2,3] ! 1

@sgraf This now checks if $3 is immediately preceded by $2 and there is no space in between. But if I put space between ! and x I still get an error if BangPatterns are not turned on. I tried that in the repl with 8.4 compiler also just to double check. This comment suggests that it should work with a space https://ghc.haskell.org/trac/ghc/ticket/13600#comment:2

Oh, that's weird. Hmm... Let me check out the code and see for myself. Warning for f ! x is not an option, of course.

I think I can cover that case too and suggest a BangPatterns since we mean to define f4 not an infix (!)

Ahh, OK, now I understand. No, we definitely shouldn't warn every time someone wants to define f ! x without -XBangPatterns. That was the whole point about being sensitive to whitespace, after all. Also, I'd never write bang patterns with a space between the bang and the binder that gets ... annotated.

So, the current behavior looks perfect to me! :)

In addition to the variants I already mentioned, Could you also add a test that validates if it works with e.g. f !(Just _)? It's kind of bogus to mean to use BangPatterns here, but I think the warning should be emitted nonetheless.

I think this is just for debugging purposes, but I want to make a mental note that this should be gone when we commit.

I suggest that instead of testing for equality, you define a helper function that pattern matches on $2 to see if it is a (L _ (HsVar _ (L _ op))) where op == bang_RDR. Kind-of like splitBang does it in RdrHsSyn.

This doesn't compile without -XBangPatterns. To be clear: We want a file that compiles without BangPatterns activated, but emits warnings only for definitions where the bang is put immediately before the second identifier, without any space.

I'd suggest the following:

-- | Should not warn
module T13600a where

f ! x = f !! x

x = [1,2,3] ! 1
  where
    f ! x = f !! x

-- | Should emit 4 warnings
module T13600b where

f !(Just x) = f !! x
f !y = head f

x = [1,2,3] ! Just 1
  where
    f !(Just x) = f !! x
    f !y = head f

This looks... weird. Have you tried to use $$ instead of <+> in your warning, like the hintBangPat error does? This puts the pretty printed expression on a new line instead of seperating the text and the message by a space.

I know I'm entirely responsible for the current warning message, but the warning could be a little less imperative, like

Did you forget to activate BangPatterns? Add a space before the bang to deactivate this warning

I introduced e here so I can use it in warnSpaceAfterBang as the second param.

This still throws an error instead of emitting a warning. Have a look at https://hackage.haskell.org/package/ghc-8.4.3/docs/Lexer.html#v:addWarning. You'll have to add the new warning to WarningFlag at some point anyway. Maybe you won't even need to synthesize e.

I also came to realize that this chunk of code probably better fits within https://github.com/ghc/ghc/blob/45befe27495b1a7bca037b6a3eedf2474a0204c8/compiler/parser/Parser.y#L3692.

if b then True else False === b

This should not fail.

How come ? It has ! glued to the function param. f !(Just x) =

It still parses as an operator. Try compiling the file with a released GHC, it's valid Haskell syntax.

Maybe name it "missing-space-after-bang" (dito the flag), because that's when it emits a warning

Could you also move the warning to Parser.y?

A little nit: I think we should remove the space before ?. Also I just went through a number of warning messages mentioning extensions and they all drop the -X prefix. Also they seem to prefer "enable" over "activate".

So: "Did you forget to enable BangPatterns?"

Link to bang patterns by

:ghc-flag:`-XBangPatterns`

Also talking about spacing is a little vague. How about

warn for missing space before the second argument of an infix definition of ``(!)`` when :ghc-flag:`-XBangPatterns` are not enabled

I presume this links to a hypthotetical ghc extension SpaceAfterBang, which doesn't exist.

Please add an empty T13600a.stderr file, so that the testrunner asserts that there were no warnings and a T13600b.stderr file that contains the expected warning messages.

The description is still too vague for my tastes. See above.

This should still be normal, not expect_broken. Also, I think the new warning should be on by default (e.g. included in -W, isn't that already the case?), so that you don't need -Wall. Otherwise, T13600a is pretty useless as a regression test, as that warning could never be triggered in the first place. I'd also suggest to turn off any other warnings (like -Wno-missing-signatures) that still are emitted after you remove -Wall.

Yeah, sorry, don't know where is my head these days. Updated now.

I think I misunderstood -W. This warning should be on by default, so please move this to standardWarnings.

If you make the new warning on by default, you can hopefully drop all the flags here and still have the same stderr. At least that's how I would expect the warning to behave.

Technically haven't a clue is also correct, it just tends to be used more in British English. I think it would be better if we dropped this change to keep the patch on-topic.

It might be a good idea to make the motivation for this warning more explicit to the user. For instance, perhaps the second sentence should rather read

If you mean to bind (!) then perhaps you want to add a space before the bang for clarity.