Squashing multiplication identity in an expression
You are doing both too much work and not enough.


You are doing too much work because you don't need to repeatedly squash units in case the first pass has returned a modified expression.
You are not doing enough work because you are not squashing the units coming from MulId rather than Lit mulId


Here is a solution which one could argue is more idiomatic as well as dealing with the two shortcomings I pointed out earlier:

We start by implementing the recursion pattern first; it is a quite common thing to do in Haskell as it helps make it easier to understand from first sight how a function works (what does this function do? Oh! It goes by recursion on the structure! Let's see what the different cases are...).

To do that, you define a function fold taking one argument per constructor of the datatype and having a given return type (here b). The arguments are functions taking the same arguments as the constructors they correspond to except that the recursive occurences of RingExpr a have been replaced by the return type b (i.e. we assume we already know the result for the recursive subcomputations!)

fold :: (a -&gt; b)      -&gt; -- Lit    :: a -&gt; RingExpr a
        b             -&gt; -- AddId  :: RingExpr a
        (b -&gt; b)      -&gt; -- AddInv :: RingExpr a -&gt; RingExpr a
        b             -&gt; -- MulId  :: RingExpr a
        (b -&gt; b -&gt; b) -&gt; -- Add    :: RingExpr a -&gt; RingExpr a -&gt; RingExpr a
        (b -&gt; b -&gt; b) -&gt; -- Mul    :: RingExpr a -&gt; RingExpr a -&gt; RingExpr a
        RingExpr a -&gt; b
fold litb addIdb addInvb mulIdb addb mulb = go
  where go (Lit a)    = litb a
        go AddId      = addIdb
        go (AddInv e) = addInvb $ go e
        go MulId      = mulIdb
        go (Add e f)  = go e `addb` go f
        go (Mul e f)  = go e `mulb` go f


The function you are looking to define is now a simple instance of the recursion pattern we just described: for the cases which are not Mul, we simply use the corresponding constructor but in the case of Mul we do the squashing:

squashMulId :: (Ring a, Eq a) =&gt; RingExpr a -&gt; RingExpr a
squashMulId = fold Lit AddId AddInv MulId Add squashing
  where
    squashing a b
      | a == Lit mulId || a == MulId = b
      | b == Lit mulId || b == MulId = a
      | otherwise                    = Mul a b